DevouchAttester with multiAttest Support & Fee Recollection #8
Conversation
|
|
||
| emit Log("attested", gasleft()); | ||
| function updateEAS(address newEASAddress) public ownerOnly { | ||
| eas = IEAS(newEASAddress); |
There was a problem hiding this comment.
will EAS Address for a chain change frequently ? If no, why do we need it as a variable?
There was a problem hiding this comment.
my thought was about uncertainty in the future, it's unlikely but it could happen. We can remove it if you want anyway - I don't think it matters that much on gas costs
There was a problem hiding this comment.
to make an unchanging value as a variable we also add the getter / setter methods. So its better to keep it as a constant for now.
src/DevouchAttester.sol
Outdated
| (bool success, ) = multisig.call{value: address(this).balance}(""); | ||
| require(success, "Failed to send Ether"); | ||
| emit Log("withdraw", gasleft()); | ||
| function withdraw() public ownerOnly { |
There was a problem hiding this comment.
anyone in GM should be able to withdraw. We have ensured that no matter who withdraws , the money always goes to multisig.
There was a problem hiding this comment.
This is Griff's requirement
There was a problem hiding this comment.
@jainkrati @mateodaza is there also a ownerOnly function to change the target recipient then? In case we need to change which multisig receives the funds. Apologies if it's already in the code somewhere
There was a problem hiding this comment.
nope, good catch - will add it 👍🏼
|
excellent @mateodaza is this ready for a merge then? |
No description provided.