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

frontend: optimize request transfer #1378

Closed
wants to merge 1 commit into from

Conversation

GabrielBuragev
Copy link
Contributor

Previously, the code didn't account for the fact that it takes a certain amount of time from the moment
a user initiates a transfer request until he actually submits it via his wallet provider.
The transfer object was fetching & storing the target chain block at the moment of triggering a "createRequest" transaction call instead of after successful submission of the transaction call.

Previously, the code didn't account for the fact that it takes a certain
amount of time from the moment
a user initiates a transfer request until he actually submits it via
his wallet provider.
The transfer object was fetching & storing the target chain block at the
moment of triggering a "createRequest" transaction call
instead of after successful submission of the transaction call.
@fredo
Copy link
Contributor

fredo commented Jan 18, 2023

Can we be sure that we do not miss the fill event if we do this change?

Specifically I'm wondering whether we fetch the target chain block after the transaction was successfully submitted to the source chain. If that's the case I believe we have a race condition between fetching the start block and actually the request already being filled.

@compojoom
Copy link
Contributor

compojoom commented Jan 18, 2023

I'm not sure if I would like to do this change now. Calling the contract function does a signer.sendTransaction(txRequest) and there are some old reports that sometimes metamask returns the hash immediately and sometimes it waits. (seems to be fixed in recent releases, but I would need more time to test)
ethers-io/ethers.js#372

Also, we always need to do getLogs for the old blocks. Whether we do 1 call that fetches only 20 blocks, or one call that fetches 500 - doesn't really matter I think.

@GabrielBuragev
Copy link
Contributor Author

@fredo true, with this change we are actually opening the possibility for such cases, therefore, i am closing this.

@GabrielBuragev GabrielBuragev deleted the frontend/optimize-transfer-block-number branch April 26, 2023 12:16
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 this pull request may close these issues.

3 participants