-
Notifications
You must be signed in to change notification settings - Fork 4
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
Andrew7234/contract accounts #428
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
LGTM since I'll be away next week (and, more importantly, since the PR is 90% there :)).
But please get @ptrus 's signoff too before merging.
creation_tx: | ||
type: string | ||
description: The Oasis cryptographic hash of the transaction that created the smart contract. | ||
creation_bytecode: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to have a discussion thread:
Reminder that you said you'd up the number of rounds in the e2e regression test. Maybe 10k is good? It takes almost exactly 2min on my machine, which feels like a decent amount of time to me. Or maybe 5k.
Might be worth a separate PR (before this one goes in) just because of all the diff noise. Your call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update this in a follow-up PR - need to also add new contract accounts to query in the e2e tests
e3b2234
to
f9cf4a2
Compare
Looks good to me now, thanks! |
[analyzer] store contract address and tx [openapi] add contract information to /runtime/accounts/{addr} [api] add contract fields to /runtime/accounts/{addr} [analyzer] store contract creation bytecode [api] show contract creation bytecode [db] add creation/runtime bytecode for evm contract accounts nit [db] standardize pkey format; remove evm contract fields [openapi] address comments address comments; simplify fields fix bad rebase linter update description
c966425
to
7e2fa47
Compare
Task
The indexer does not differentiate between "normal" accounts and accounts that represent a smart contract in EVM. For thet latter, we could expose an additional (structured) field in the API, with information like
Side task: We promise to show the address of the newly-created contract in our openapi docs promise as the
to
field: https://github.com/oasisprotocol/oasis-indexer/blob/46e5f0e8f65b414d7e6ce0b2037b223a91097540/api/spec/v1.yaml#L2082-L2082. Populate it. (Note: Maybe Peter Us will do it; coordinate)Implementation: A possible approach is to expand/reuse the
evm_tokens
DB table (which currently contains metadata about contracts that look like ERC-XX tokens) into a more generalevm_contracts
table, with the above additional columns. Or else (preferred; discussed between Mitja and Peter Us) we leaveevm_tokens
focused on token-like metadata, and split off the remaining contract metadata into a new table.This PR
This PR solves part of the task, by adding the basic structure for contract account information when a runtime account address happens to also represent a smart contract.
Several fields; notably
bytecode
,abi
, andverified
are left empty or default to false currently. These fields will be populated by the evm tokens/contract analyzer in a future PR.Testing:
Ran for ~2k blocks; successfully tested that contract accounts on both emerald and sapphire behave as expected:
Side note*: The additional fields are not currently included in the
run.sh
api test cases since there are no contract accounts within the first 100 blocks.. filing a ticket to expand this.Discussion copied from elsewhere:
creation_tx
field can be populated by the runtime analyzer, but from my understanding thebytecode
andverified
fields will need to be populated by the evm_token_analyzer or something similar. That seems to overlap with evm/sourcify: support EVM contract verification via Sourcify #429 to download the ABIs of existing contracts - so is populatingbytecode/verified
out of scope for this ticket?