Skip to content
This repository was archived by the owner on Aug 27, 2025. It is now read-only.

[master] Otterscan final apis#3634

Merged
n-hutton merged 36 commits intomasterfrom
otter
Jun 7, 2023
Merged

[master] Otterscan final apis#3634
n-hutton merged 36 commits intomasterfrom
otter

Conversation

@n-hutton
Copy link
Copy Markdown
Contributor

@n-hutton n-hutton commented May 23, 2023

Add the missing apis that allow otter to search for transactions that have touched a certain address etc.

Needs Zilliqa/evm#5

This will only work if you point otterscan at a node/iso server that has ARCHIVAL_LOOKUP_WITH_TX_TRACES enabled. In this special case, it will start to collect otter logs from evm and tx execution and write them to disk.

It piggybacks off the tx_traceTransaction functionality which was already there. The tx trace stuff actually keeps a rolling cache on disk so for full permanence the otter stuff had to be split into its own storage.

There are some performance considerations: the disk space for the otter search before/after could become very large over time, and queries of an address with a lot of traffic could also take a very long time to parse (or even crash the seedpub due to OOM error).

Also, the evm is now returning more text per IPC call, which makes it slower. This can cause evm tests to time out and could be an issue if the seedpub 'falls behind'.

Fortunately all of this stuff in prod is essentially only on a dedicated seedpub.

JamesHinshelwood and others added 10 commits April 25, 2023 18:06
This is an API which returns the currently implemented version of
the Otterscan API (`ots_*`). The frontend checks this number is
high enough before trying to do anything else. The current
minimum version in the frontend is `8`, so we return `8`.
To do this, we add a new `contractCreators` database which stores
a map of contract addresses to the transaction hash in which that
contract was created. This allows us to look up the creator of the
contract in most cases by using the `from` address of the creation
transaction.

Note that this doesn't work for contracts which have been deployed
by other contracts. The API documentation specifically mentions we
should support this, so I've left a `FIXME` for now. To do this,
we'll probably want to extend the data stored when we create a
contract to also track the immediate caller (and thus the contract
creator).
@github-actions github-actions Bot changed the title Otterscan final apis [master] Otterscan final apis May 23, 2023
}
}

fn get_create_address(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

now a requirement of handler via the evm PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is get_create_address actually used anywhere in this PR? I can't find any usages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the other PR that adds evm functionality it adds it as a handler fn and is used there impl<'a> Handler for CpsExecutor<'a>


match exit_reason {
evm::ExitReason::Succeed(_) => {}
evm::ExitReason::Revert(_) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

when tx fails, capture error blob

Comment thread evm-ds/src/main.rs
struct LoggingEventListener {
call_tracer: Vec<CallContext>,
raw_tracer: StructLogTopLevel,
otter_internal_tracer: Vec<InternalOperationOtter>,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

reuse transaction trace struct


cd tests/EvmAcceptanceTests/
npm install
DEBUG=true MOCHA_TIMEOUT=20000 npx hardhat test
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this has effect of not interleaving the hardhat output with the iso server output, it just dumps it after

Copy link
Copy Markdown
Contributor

@JamesHinshelwood JamesHinshelwood left a comment

Choose a reason for hiding this comment

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

Overall, LGTM.

Probably makes sense for someone more qualified in C++/ZQ1 to have a review too. @Steve-White-UK ?

}
}

fn get_create_address(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is get_create_address actually used anywhere in this PR? I can't find any usages.

Comment thread evm-ds/src/evm_server_run.rs Outdated
@JamesHinshelwood
Copy link
Copy Markdown
Contributor

Will test this branch manually too

n-hutton and others added 3 commits May 25, 2023 15:47
Co-authored-by: James Hinshelwood <JamesHinshelwood@users.noreply.github.com>
@Steve-White-UK Steve-White-UK requested review from Steve-White-UK and removed request for Steve-White-UK May 25, 2023 17:33
Copy link
Copy Markdown
Contributor

@Steve-White-UK Steve-White-UK left a comment

Choose a reason for hiding this comment

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

I so no reason not to merge to master if James is happy with that.


Json::Value parsed;

auto const item = trace_json["otter_addresses_called"];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could it be just auto const parsed = ...?

@n-hutton n-hutton merged commit 199b8da into master Jun 7, 2023
@n-hutton n-hutton deleted the otter branch June 7, 2023 14:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants