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

Ensure frontend is setup to handle multiple relayers #329

Closed
mds1 opened this issue Apr 12, 2022 · 5 comments
Closed

Ensure frontend is setup to handle multiple relayers #329

mds1 opened this issue Apr 12, 2022 · 5 comments
Labels
app A change related to the Umbra frontend enhancement New feature or request

Comments

@mds1
Copy link
Collaborator

mds1 commented Apr 12, 2022

Do we get a regular tx hash or a tx ID back from OZ relayers used on polygon + L2s? This block of code assumes we get a tx hash on polygon, and an ID everywhere else, but that seems incorrect? (this comment is unrelated to the diff)

Originally posted by @mds1 in #319 (comment)

With ITX (on Mainnet/Rinkeby), their API returns a custom hash and you have to subsequently query their endpoint to figure out the list of actual network transaction hashes. That is what we pass forward from Umbra API.

With Defender, we get back the hash of the first transaction submitted, and that is what we pass forward from Umbra API. Defender also gives you a separate, custom Identifier which can use to query their API, to find out if that first transaction has been replaced or cancelled. We currently don't expose this in any response from Umbra API.

In practice, since we're using this on Polygon and using fast, this virtually never happens, and so the hash we get back ends up being the hash of the transaction that is ultimately mined. This should also be the case on Optimism and Arbitrum.

If we ever started using Defender on mainnet, we might have to revisit this assumption.

Originally posted by @apbendi in #319 (comment)

@mds1 mds1 added the app A change related to the Umbra frontend label Apr 12, 2022
@garyghayrat
Copy link
Member

  • Because all networks are now using OZ defender set to 'fast', it is relatively safe to assume the tx hash returns will be mined and to display this to the user.
  • The relay withdraw window should be updated to include Etherscan link to the tx hash.

@garyghayrat garyghayrat added the enhancement New feature or request label Jul 6, 2022
@garyghayrat garyghayrat added this to Umbra Jul 6, 2022
@garyghayrat garyghayrat moved this to 📋 Backlog in Umbra Jul 6, 2022
@mds1
Copy link
Collaborator Author

mds1 commented Jul 7, 2022

Since we now use OZ everywhere, I think original scope of this issue is outdated and the issue can be closed. But we don't have support for properly handling replacement txs with OZ—I'm not sure if they have an endpoint we should use to fetch the latest tx hash or some other approach, but either way it probably could be a separate issue.

// This is a regular transaction hash, though it's possible OZ Defender will replace it to ensure it gets
// included quickly, in which case the frontend would not automatically reflect when the relay was successful.
// Because we relay with the "fast" setting, this is unlikely to be the case.
window.logger.info(`Relayed with transaction hash ${relayTransactionHash}`);
const receipt = await provider.value.waitForTransaction(relayTransactionHash);
window.logger.info('Withdraw successful. Receipt:', receipt);

The relay withdraw window should be updated to include Etherscan link to the tx hash.

What is this referring to? My guess is that the tx notification window in the top right that shows tx status does not let you click it to view the withdraw tx?

@mds1
Copy link
Collaborator Author

mds1 commented Feb 6, 2023

But we don't have support for properly handling replacement txs with OZ—I'm not sure if they have an endpoint we should use to fetch the latest tx hash or some other approach, but either way it probably could be a separate issue.

@apbendi this is probably a question for you, is this something we need to support / track in an issue?

The relay withdraw window should be updated to include Etherscan link to the tx hash.

What is this referring to? My guess is that the tx notification window in the top right that shows tx status does not let you click it to view the withdraw tx?

@garyghayrat bumping this question

@garyghayrat
Copy link
Member

The relay withdraw window should be updated to include Etherscan link to the tx hash.

What is this referring to? My guess is that the tx notification window in the top right that shows tx status does not let you click it to view the withdraw tx?

@garyghayrat bumping this question

Seems to have been solved with #381

@garyghayrat
Copy link
Member

Opened a new issue to track. #634

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Umbra Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app A change related to the Umbra frontend enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

2 participants