Conversation
| */ | ||
| function executeSystemWithdrawals(uint64[] calldata _amounts, address[] calldata _addresses) external { | ||
| require( | ||
| _msgSender() == SYSTEM_WITHDRAWAL_EXECUTOR || _msgSender() == _admin(), |
There was a problem hiding this comment.
Why also allow for _admin address?
There was a problem hiding this comment.
That is more for test purposes. I mean anyway the admin have rights to do everything with this contract, but this way we have the opportunity to conveniently test this contract.
There was a problem hiding this comment.
Has the function name defined in the spec been changed permanently?
There was a problem hiding this comment.
No. I just used more declarative name. For sure we can change it if it is required.
There was a problem hiding this comment.
@Barichek I like this better as I prefer the function name to be a verb. So, either this one or just withdraw. And since this function is called from the execution client, I need to know what name to use. If we stick with this one, the spec needs to be updated.
There was a problem hiding this comment.
I can update the spec with executeSystemWithdrawals. While admin can do anything, having it allowed here explicitly is a bit unclean. Can you find another approach to become the system executor on testing?
… manual failed withdrawal processing
dapplion
force-pushed
the
ib-withdrawal-contract-part
branch
from
dacd43c to
f2eb640
Compare
|
| * @param _amounts Array of amounts to be withdrawn. | ||
| * @param _addresses Array of addresses that should receive the corresponding amount of tokens. | ||
| */ | ||
| function executeSystemWithdrawals( |
There was a problem hiding this comment.
An idea to consider. If we would change this function to return a boolean flag indicating whether withdrawals were succeeded or not, an execution client could log a warning hinting to check the contract for details. This could improve the user experience a little at the cost of a slight gas usage increase because of the flag handling. Not entirely sure though whether it's worth that.
There was a problem hiding this comment.
Not sure of its value either, won't be opposed to it. It's not actionable info for a user, same way failed txs from the network are not logged.
|
Okay |
Skip bad withdrawals
Test bad withdrawals
* wip: remove mGNO * test: fix remaining tests * chore: update error message * feat: add `unwrapTokens` function * wip: upgradeToAndCall * chore: remove package-lock and reset yarn.lock * Add division on withdraw --------- Co-authored-by: dapplion <35266934+dapplion@users.noreply.github.com>
5 participants
Enable withdrawals on Gnosis chain, conforming to spec https://github.com/gnosischain/specs/blob/master/execution/withdrawals.md
unwrapTokensonce to unwrap all deposited mGNO prior to the upgrade