-
Notifications
You must be signed in to change notification settings - Fork 233
feat/native-sync-vault-composer #1747
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
base: main
Are you sure you want to change the base?
Conversation
… override Signed-off-by: shankar <[email protected]>
…d sendRemote Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Co-authored-by: Tino Martínez Molina <[email protected]>
9c5ec72 to
f5e888a
Compare
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
…ous operations (#1778) Signed-off-by: shankar <[email protected]>
| If you are staking in Stargate's native pools you will need a slightly different setup. As the asset is the native token there is not | ||
| really an OFT or an ERC20 address. So for the asset OFT and ERC20 address you need to supply the Native Pool and `0x0`(or some other 0 value hex string). | ||
| The Native Pool implements the OFT interface, and the `0x0` address will let the library know that it should handle it as a native pool. Other than that | ||
| it is the same. If you are using wETH, you can supply arguments normally. This is just for ETH or other native tokens specifically. |
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.
One line?
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.
@arekLZ can you help with this?
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.
Run prettier on this file?
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.
cc: @arekLZ
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.
What styling is off?
| it.skip('Redeem A->B->B', async () => { | ||
| const srcChain = chainInputs['arbitrum']! | ||
| const dstChain = chainInputs[hubChain]! | ||
| describe.skip('OFT Vaulting', () => { |
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.
Why are those tests skipped? Should we be adding a comment?
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 believe its cuz these are vape tests that actually send transactions. @arekLZ can confirm
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.
These are not tests that regularly run. They are more for verification and regression and should be run manually. They actually send transactions as Shankar mentioned.
| if (hexToBigInt(tokenAddress) === 0n) { | ||
| // If sending native tokens, we need to account for slippage | ||
| const amountDst = await this.getOutputAmount(input) | ||
| const slippageAmount = (amountDst * BigInt(Number(slippage.toFixed(3)) * 1000)) / 1000n | ||
| minAmountLD = amountDst - slippageAmount | ||
| } |
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.
Why do we only account for slippage here for native tokens?
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.
cc: @arekLZ
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.
Only native tokens should have slippage. If we are sending OFTs, there should be 0 slippage.
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Co-authored-by: Tino Martínez Molina <[email protected]>

This PR introduces composers for StargateAssets:
VaultComposerSync
A refactor of
VaultComposerSyncwas made (and also required) to improve the code that would be created for these assets since they break several assumptions:OFT.token == vault.asset()breaks with PoolNativeOFT.approvalRequired()returnsTruewhen approval is not required - StargatePoolNativeUpdating certain functions to also being
virtualand splitting_send()into_sendLocal()and_sendRemote()are non-functional changes that let us override_sendRemote()in the StargateAssetComposers.The constructor now calls an internal function
_initializeAssetToken(assetOFT,vault)and this is overridable which helps us work around (1) and (2)VaultComposerSyncPoolNative
Stargate's nativepool asset's inner token is ETH - address(0) while the
vault.asset()is WETH. This means that we need to change the functionality to wrap ETH that we receive from the NativePool when talking to the Vault. On refunds due to failed deposits (Pool gives us ETH) we want to have identical behavior against a redemption (vault gives us WETH). We overridereceive()to wrap ETH to WETH when the sender is the Pool. This only happens onlzReceive().Stargate's NativePools also has
approvalRequired()set toTruewhich is incorrect.These are addressed by overriding
_initializeAssetToken()to validate the token and approve WETH.