Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dxdaoTimeLock #754

Draft
wants to merge 2 commits into
base: arc-factory
Choose a base branch
from
Draft

dxdaoTimeLock #754

wants to merge 2 commits into from

Conversation

orenyodfat
Copy link
Contributor

No description provided.

ben-kaufman
ben-kaufman previously approved these changes May 20, 2020
Copy link
Contributor

@dkent600 dkent600 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this contract name prefixed with "dxDao"? Does it have no possible more widespread use?

// solhint-disable-next-line not-rely-on-time
require(releaseTime < now, "cannot withdraw before releaseTime");
// solhint-disable-next-line avoid-call-value
(bool success, ) = owner.call.value(address(this).balance)("");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not msg.sender.transfer(address(this).balance) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because transfer is limited in gas. it might work for dxdao avatar though no promise for other contracts which consume gas on the fallback function.
https://diligence.consensys.net/blog/2019/09/stop-using-soliditys-transfer-now/

public
returns(bool success, bytes memory returnValue) {
// solhint-disable-next-line avoid-low-level-calls
(success, returnValue) = _contract.call(_encodedABI);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why instead of sending the _encodeABI we execute the withdraw function with the signature that we already know it will have?

(success, returnValue) = _contract.call(abi.encodeWithSignature("withdraw()"));

Doing this we make sure that the ONLY function that can be executed is the withdraw()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a public generic function which can call any contract specific Abis.
this is just a test helper contract and is not part of any dao.

@AugustoL
Copy link

Why is this contract name prefixed with "dxDao"? Does it have no possible more widespread use?

yeah, right? I guess the idea is to have this feature to all DAOs..

@orenyodfat
Copy link
Contributor Author

Why is this contract name prefixed with "dxDao"? Does it have no possible more widespread use?

yeah, right? I guess the idea is to have this feature to all DAOs..

I am not sure yet . We might have it specific to dxdao by hard coded its address and release time.though might complicate the tests.
yea. lets make it general

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants