-
Notifications
You must be signed in to change notification settings - Fork 1
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
2512 PufferWithdrawalManager #39
Conversation
|
||
/** | ||
* Request a withdrawal of the given amount to the given address, with a permit. | ||
* @param walletAddress The account address to request the withdrawal for. |
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.
Would be good if we could leave a space between the param and the description to make it more readable.
* @returns The transaction hash of the withdrawal. | ||
*/ | ||
public async requestWithdrawal(walletAddress: Address, amount: bigint) { | ||
return await this.getContract().write.requestWithdrawal( |
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.
As a general practice across the SDK, the write functions should return transact
and estimate
so they SDK consumers can estimate gas if they need to.
getContract: jest.fn(), | ||
})); | ||
jest.mock('./erc20-permit-handler'); | ||
const mockErc20PermitHandler = jest.mocked(ERC20PermitHandler); |
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.
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.
Just one more comment.
walletAddress: Address, | ||
withdrawalIdx: bigint, | ||
) { | ||
return await this.getContract().write.completeQueuedWithdrawal( |
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.
This also needs the transact
and estimate
.
Exposing functionality of the 2-step withdrawal manager in the SDK.
These are the likely usable functions, but we can add more later.
Not testable yet since various aspects are still WIP on the smartcontract side.