-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add methods for batch withdrawals requests and claiming #513
Add methods for batch withdrawals requests and claiming #513
Conversation
contracts/0.8.9/WithdrawalQueue.sol
Outdated
/// @notice Request withdrawal of the provided token amount in a batch | ||
/// NB: Always reverts | ||
function requestWithdrawalBatch(uint256[] calldata, address[] calldata) | ||
struct WithdrawalRequestInput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice idea 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any drawbacks in reusing these structs in all the methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can even get rid of the singular methods at all. I like this idea more and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DiRaiks fyi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! 👍
Left some considerations 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should get rid of singular request/claim in favor of plural and get rid of the Batch
in the names as well. It will give a smaller surface and more consistency.
contracts/0.8.9/WithdrawalQueue.sol
Outdated
/// @notice Request withdrawal of the provided token amount in a batch | ||
/// NB: Always reverts | ||
function requestWithdrawalBatch(uint256[] calldata, address[] calldata) | ||
struct WithdrawalRequestInput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any drawbacks in reusing these structs in all the methods?
contracts/0.8.9/WithdrawalQueue.sol
Outdated
/// @notice Request withdrawal of the provided token amount in a batch | ||
/// NB: Always reverts | ||
function requestWithdrawalBatch(uint256[] calldata, address[] calldata) | ||
struct WithdrawalRequestInput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can even get rid of the singular methods at all. I like this idea more and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM mostly, left some additional suggestions
No description provided.