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

feat: Emit Aurora event in ETH exit precompile. #245

Closed
paouvrard opened this issue Aug 20, 2021 · 4 comments · Fixed by #288
Closed

feat: Emit Aurora event in ETH exit precompile. #245

paouvrard opened this issue Aug 20, 2021 · 4 comments · Fixed by #288
Labels
C-enhancement Category: New feature or request P-high Pririoty: high

Comments

@paouvrard
Copy link
Member

In order to improve the bridge UI experience, we need to query for all the transfers initiated by an account on Aurora.
When withdrawing a bridged ERC-20, withdrawToEthereum or withdrawToNear are called and emit a Transfer event when _burn is executed:

_burn(_msgSender(), amount);
We can then query and filter all those events to find the transfers.

However when withdrawing ETH, no event is emitted. Would it be possible to also emit an exit event for ETH from the precompile ?

@paouvrard paouvrard added C-enhancement Category: New feature or request help wanted labels Aug 20, 2021
@paouvrard
Copy link
Member Author

Above I mentioned that ERC-20 exits could be found by querying burn events: Transfer(account, address(0), amount).

Since the same burn event is emitted for withdrawToEthereum and withdrawToNear, we can check if the Aurora tx data contains the sigHash of one of those those 2 functions to differentiate the type of exit. Note that this method would not work for specific multisig transactions which record approvals on-chain (the tx data would not contain the sigHash in that case).

This is quite a rare scenario as multisig wallets like Argent gather signatures off-chain and I don't think these types of legacy gnosis on-chain signatures are common (except in DAOs). So we may consider emitting a custom event for ERC-20 exits also, but it doesn't seem necessary at this time.

@artob
Copy link
Contributor

artob commented Sep 8, 2021

@sept-en Please work this into your near-term roadmap and communicate a schedule to the Product team.

@sept-en
Copy link
Contributor

sept-en commented Sep 8, 2021

@artob Ok, sure.

@joshuajbouw joshuajbouw added this to the v1.7.0 milestone Sep 13, 2021
@joshuajbouw joshuajbouw added the P-high Pririoty: high label Sep 13, 2021
@joshuajbouw joshuajbouw removed this from the v1.7.0 milestone Sep 14, 2021
@birchmd birchmd linked a pull request Sep 28, 2021 that will close this issue
@birchmd
Copy link
Member

birchmd commented Oct 8, 2021

Fixed in #288 + #292

@birchmd birchmd closed this as completed Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: New feature or request P-high Pririoty: high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants