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

Implementing Otterscan support #5414

Merged
merged 43 commits into from
Aug 8, 2023
Merged

Implementing Otterscan support #5414

merged 43 commits into from
Aug 8, 2023

Conversation

naps62
Copy link
Contributor

@naps62 naps62 commented Jul 16, 2023

Motivation

The people want Etherscan for their anvil nodes.

Solution

Implements the Otterscan JSON RPC namespace, along with all the quirks that were found to be needed.
Some of the code is a bit messier than I'd like,

This is still a work in progress, as I have two endpoints to implement still, but they should be easy (famous last words) compared to a lot of the others, so I'm opening this ahead of time to gather feedback.
The code is also a bit messier than I'd like, but I was trying to touch as little as possible on Backend and other modules. maybe there's some patterns or utility function I missed in the codebase that could have helped, so feedback is much appreciated!

This was also a bit tricky for a couple of reasons:

  • Otterscan's endpoints are inherently hard to compute with existing data (otherwise they wouldn't be needed in the first place). They solve things that the original RPC spec didn't account for (most notably, listing historical transactions by address). For anvil, this is mostly a non-issue, as we can choose to traverse all the blocks & traces in-memory. Would be interesting to test this on an node with a heavy data-set though;
  • After having gone through the spec, it seems to not be as well though-out as it could, and in some cases outdated compared to their code. Some of the design decisions behind it are a bit awkward to implement (see Outdated spec? otterscan/otterscan#1081).

todo list

  • erigon_getHeaderByNumber
  • ots_getApiLevel
  • ots_getInternalOperations
  • ots_hasCode
  • ots_traceTransaction
  • ots_getTransactionError
  • ots_getBlockDetails
  • ots_getBlockDetailsByHash
  • ots_getBlockTransactions
  • ots_searchTransactionsBefore
  • ots_searchTransactionsAfter
  • ots_getTransactionBySenderAndNonce
  • ots_getContractCreator
  • fixing all TODO notes in codebase

image
image
image
image

Adds anvil support for Otterscan's custom RPC endpoints.

This is still a work in progress, as I have two endpoints to implement
still, but they should be **easy** (famous last words) compared to a lot
of the others, so I'm opening this ahead of time to gather feedback.

This was a bit tricky for a couple of reasons:
* Otterscan's endpoints are inherently hard to compute with existing
  data (otherwise they wouldn't be needed in the first place). They
  solve things that the original RPC spec didn't account for (most
  notably, listing historical transactions by address). For anvil, this
  is mostly a non-issue, as we can choose to traverse all the blocks &
  traces in-memory. Would be interesting to test this on an node with
  a heavy data-set though;
* After having gone through the spec, it seems to not be as well
  though-out as it could, and in some cases outdated compared to
  their code. Some of the design decisions behind it are a
  bit awkward to implement (see otterscan/otterscan#1081).
@naps62
Copy link
Contributor Author

naps62 commented Jul 18, 2023

@Evalir couple questions:

  • I took a stab at one of the tests, but not sure I used the write approach (maybe there's a more appropriate test for this layer that I could base it on?)
  • I'm not yet sure about a couple of the endpoints. left a few TODOs in there where I still have questions. would appreciate feedback

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey hey! tysm for this—this is awesome.

i have some comments/clarifications so we can keep this going—i tried to check for correctness but the best way to keep testing is probably to keep testing directly with otterscan and see what it likes or doesn't like.

cc @mattsse beefy pr so ideally we'd have two eyes on this

/// Otterscan's `ots_getApiLevel` endpoint
#[cfg(feature = "otterscan")]
#[cfg_attr(feature = "serde", serde(rename = "erigon_getHeaderByNumber"))]
ErigonGetHeaderByNumber(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be called so? If so, can we add a bit of context as to why it has erigon naming? not super familiar with otterscan so just wanna make sure we have this documented 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I documented the impl EthApi functions, but now that you mention, probably most of those comments belong here instead

As for this particular one, it's otterscan being a bit sneaky and making an erigon-specific call on load to "check if we're connected to an erigon node". In practice I think they could achieve the same result (show a proper error msg when the node isnt' supported) by using ay other ots_ endpoint, but oh well.

Perhaps I'll change that upstream later. for now it's reported here, waiting for input: otterscan/otterscan#1081

anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/src/eth/mod.rs Outdated Show resolved Hide resolved
anvil/src/eth/otterscan.rs Outdated Show resolved Hide resolved
anvil/src/eth/otterscan.rs Outdated Show resolved Hide resolved
anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/src/eth/otterscan.rs Outdated Show resolved Hide resolved
anvil/tests/it/otterscan_api.rs Outdated Show resolved Hide resolved
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another batch of comments, but this should be real close

@mattsse would love your eyes on this!

) -> Result<Option<Block<TxHash>>> {
node_info!("ots_getApiLevel");

// TODO: probably only return blocks from before the fork?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you determine what this should be? IMO it's fine if it's only post-fork.

anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/src/eth/otterscan.rs Outdated Show resolved Hide resolved
anvil/src/eth/otterscan.rs Outdated Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fantastic.

all of this looks very good already, only have a few needs re organization.

anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/src/eth/api.rs Outdated Show resolved Hide resolved
anvil/tests/it/otterscan.rs Outdated Show resolved Hide resolved
@naps62
Copy link
Contributor Author

naps62 commented Aug 7, 2023

@Evalir @mattsse thanks for all the help.
Pretty sure now I handled all missing tests, and your feedback (including some comments about removing unwrap() that had gone outdated)

Only thing left is this now, which I can't seem to figure out. would appreciate a hand here whenever possible 🙏🏼

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is beautiful—I'd just like docs for the things that don't have any.

Will look at solving the last todo as well!

anvil/src/eth/otterscan/types.rs Show resolved Hide resolved
anvil/src/eth/otterscan/types.rs Show resolved Hide resolved
anvil/src/eth/otterscan/types.rs Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing, this will be super useful

haven't reviewed the new handler impls too closely but all they do is add additional features so should be fine

anvil/tests/it/otterscan.rs Outdated Show resolved Hide resolved
@Evalir Evalir marked this pull request as ready for review August 7, 2023 19:00
@naps62
Copy link
Contributor Author

naps62 commented Aug 8, 2023

@Evalir I believe everything's now done!

(I'll later create an issue on the book repo, and probably work on it as well)

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is beautiful. Amazing work @naps62 — it was such a pleasure reviewing this!

@Evalir Evalir merged commit e4ac420 into foundry-rs:master Aug 8, 2023
trajan0x added a commit to synapsecns/sanguine that referenced this pull request Aug 18, 2023
dan-aztec added a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 5, 2023
adds a local otterscan block explorer for the anvil ethereum mainnet,
helps to see what's happening on the base network. (they[ recently added
support](foundry-rs/foundry#5414) for otterscan
in anvil)

the transactions/calls are not decoded, so right now it's primarily
useful if we are using portal contracts - hard to tell what the rollup
contracts are doing.

sets the local otterscan at http://localhost:5100/


https://github.com/AztecProtocol/aztec-packages/assets/142251406/0dd83352-103b-4b75-8fb6-baff5f0eee6b
AztecBot pushed a commit to AztecProtocol/docs that referenced this pull request Oct 6, 2023
adds a local otterscan block explorer for the anvil ethereum mainnet,
helps to see what's happening on the base network. (they[ recently added
support](foundry-rs/foundry#5414) for otterscan
in anvil)

the transactions/calls are not decoded, so right now it's primarily
useful if we are using portal contracts - hard to tell what the rollup
contracts are doing.

sets the local otterscan at http://localhost:5100/


https://github.com/AztecProtocol/aztec-packages/assets/142251406/0dd83352-103b-4b75-8fb6-baff5f0eee6b
@TangMonk
Copy link

Otterscan works on anvil now?

@naps62 naps62 deleted the ots branch April 29, 2024 16:23
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.

4 participants