Skip to content
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

Bridge Integration: Withdraw: regen deposit address when changing account #968

Closed
jonator opened this issue Nov 7, 2022 · 1 comment · Fixed by #1056
Closed

Bridge Integration: Withdraw: regen deposit address when changing account #968

jonator opened this issue Nov 7, 2022 · 1 comment · Fixed by #1056

Comments

@jonator
Copy link
Member

jonator commented Nov 7, 2022

Problem

With the Axelar bridge integration, while a user is depositing, changing the account in MetaMask causes no issues since generating a deposit address only takes in the destination address (on Osmosis in this case). So, it doesn't matter which account funds are sent from. However, when withdrawing, switching the account in MetaMask (the destination address) causes the generated deposit address to no longer be valid. Sending funds to it would transfer the funds to the address selected while the modal was first opened.

Proposed Solution

Instead, we should handle this by regenerating the account address every time and only when address in the snippet below changes, causing this useEffect hook to re-run and invoke the Axelar SDK deposit address API. I faintly remember having this enabled before, but I added the last check (!depositAddress) to prevent the doGen from being run when other deps besides just address is changed. AKA instead of getting the right deps arrays in this and subsequent memo hooks right, I added this check and introduced this bug. Perhaps an additional if statement should be added to just handle the address changing, rather than the component mounting which is already in the snippet.

useEffect(() => {
if (address && generateOnMount && !depositAddress) {
doGen().catch((e) => console.error(e));
}
}, [address, generateOnMount, depositAddress, doGen]);

@jonator
Copy link
Member Author

jonator commented Nov 7, 2022

Alternative/fallback solution if nothing else works. Keep track of the address the Axelar deposit address was generated for, and if the MetaMask account is different, display an error and disable button.

Edit, perhaps for some number of re-queries, we could rerun the address generation if the current address is changed from the recently generated-for destination address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant