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

Settle on a web3 library #209

Closed
spalladino opened this issue Apr 6, 2023 · 5 comments · Fixed by #408
Closed

Settle on a web3 library #209

spalladino opened this issue Apr 6, 2023 · 5 comments · Fixed by #408

Comments

@spalladino
Copy link
Collaborator

Viem has added support for local signing, so we are free to use it. Decide on it vs the local ethereum.js.

@spalladino spalladino added this to A3 Apr 6, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Apr 6, 2023
@benesjan
Copy link
Contributor

benesjan commented Apr 12, 2023

I asked how would viem handle the load-balancer-caused Infura issue here and the answer is that in case the node would support filters then it should work. The potential issue with the filter approach is if there would be a maximum allowed block range set by Infura which ChatGPT says is 10000 blocks but I couldn't find on Infura website so the model might be hallucinating.

In case the node didn't support filters it would cause issues.

Viem is a wrapper around the JSON_RPC methods and for this reason we could use the eth_getLogs method and use our own sync loop with Viem.

I would avoid using filters as they just seem to be too implementation specific. The approach we took with Aztec Connect of using eth_getLogs and requesting the events from a block range <num_of_last_block_containing_log + 1; latest_block_num> instead of <num_of_latest_block_from_last_request + 1; latest_block_num>.

@spalladino
Copy link
Collaborator Author

+1 to not relying on filters, but note that querying events up to latest_block_num makes the implementation susceptible to reorgs (basically anything that queries up to the latest block, whether it is polling, filters, or websockets, will have this problem).

An easy solution is to run two epochs behind the tip of the chain, to ensure finalization, but this introduces a massive delay. The other option is actually reacting to reorgs. Both filters and websockets have built-in mechanisms for that: they report logs with a "deleted" flag when they are removed. But that requires a persistent connection, which may not be easy to guarantee.

When polling, you can check for changes in the blockhash of the last known block number, and that will tip you off that you've hit a reorg. I got a sample implementation of this here, but probably the most robust solution is using something like https://github.com/ethereumjs/ethereumjs-blockstream that keeps a local stream of blocks, checks for reorgs, and triggers events whenever it detects one. And it's provider-agnostic.

@benesjan
Copy link
Contributor

benesjan commented Apr 12, 2023

@spalladino Good point. In Aztec Connect we used the easy solution. As I am looking at the Terraform config the MIN_CONFIRMATIONS param was set to 3 in prod. We've never had any issues but it would probably be a good idea to revisit this because given that we will have decentralized sequencer from day one we will need to strive for higher robustness. OTOH from what I know there are basically no significant reorgs since the merge.

@spalladino
Copy link
Collaborator Author

True, I scolled past 50 pages of https://etherscan.io/blocks_forked and the deepest reorg I found was 2. We don't want to get bit by this the day there is an actual reorg, but it's also something we may not need to work on atm.

@spalladino
Copy link
Collaborator Author

Just keep in mind that setting MIN_CONFIRMATIONS to anything greater than zero may break devnet setups with automine (ie hardhat network or anvil instances that only mine new blocks when there are new txs, so if there's no activity no new blocks get mined, and you never reach the number of confirmations needed).

@github-project-automation github-project-automation bot moved this from Todo to Done in A3 May 4, 2023
ludamad pushed a commit that referenced this issue Jul 14, 2023
@iAmMichaelConnor iAmMichaelConnor removed this from A3 Jul 26, 2023
codygunton pushed a commit that referenced this issue Jan 23, 2024
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.

3 participants