-
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
EVM parsing functions with ABI #421
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.
Super neat! IMO this needs some JSON compatibility tests (see thread inline) and fmt.Printf
replaced with asserts ... and it's ready.
I'm assuming you were planning to integrate these funcs into the analyzer in a separate PR?
ethCommon "github.com/ethereum/go-ethereum/common" | ||
) | ||
|
||
func EVMParseData(data []byte, contractABI *abi.ABI) (*abi.Method, []interface{}, error) { |
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.
It's not clear what Go types will be used in the elements of []interface{}
.
Looking at the ethereum-go tests, it's "whatever makes sense" https://github.com/ethereum/go-ethereum/blob/9231770811cda0473a7fa4e2bccc95bf62aae634/accounts/abi/abi_test.go#L810-L813 but it's not clearly documented. Going more into the weeds, it is one of the many possible return values of toGoType().
Perhaps we don't need to care much, as long as serializes cleanly into JSON (which is the format we use in the DB, and in our API).
Please test at least with large integers; ethereum-go uses *big.Int
, which is reasonable, but does not support pgx-compatible serialization. We'll be converting everything into JSON manually before throwing it at pgx, so we're probably fine?
Anyway, it would be good to have some EVM parsing tests not just into Go-native types, but into JSON. If you can find/generate example inputs for wacky types (e.g. multi-dimensional array of tuples of (bigint, address)), all the better.
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.
😬 yeah should check json
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.
yeah it's *big.Int and it tries to put it into json as just the digits 😱
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.
alright tests are added
we don't have the ABIs being collected yet, so I don't have a place to integrate this yet |
5fd593b
to
39de649
Compare
You can integrate with the existing ERC-20 parsing; it will make it easier to then later generalize from ERC-20 to [any contract whose ABI we have]. |
we have the ERC-20 ABI, but we don't have a piece of data telling us which contract to use which ABI for |
struct O {
uint16 n;
string s;
}
interface Varied {
function test(
int8 i8,
uint8 u8,
int256 i,
uint256 u,
bool b,
// not supported in go-ethereum
// fixed128x18 f18,
// ufixed128x18 uf18,
bytes32 b32,
address a,
function (uint16) external returns (uint16) f,
uint16[2] calldata xy,
bytes calldata buf,
string calldata s,
uint16[] calldata l,
O calldata o
) external;
}
[-1,1,-1,1,true,[1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1],"0x0101010101010101010101010101010101010101",[1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,2,2,2,2],[1,1],"AQ==","a",[1],{"n":1,"s":"a"}] eth (solidity) -> go (go-ethereum) -> JSON |
current complaints:
|
[-1,1,"-1","1",true,"AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE=","0x0101010101010101010101010101010101010101","AQEBAQEBAQEBAQEBAQEBAQEBAQECAgIC",[1,1],"AQ==","a",[1],{"n":1,"s":"a"}] custom converting possibly weird philosophically: uint8[] and uint8[n] I believe will also be base64 |
5496614
to
e90ef11
Compare
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! Largely LGTM for the current changes.
Also, to revisit the deeper-integration conversation (stupid github for preventing PR-level threads):
I'm assuming you were planning to integrate these funcs into the analyzer in a separate PR?
we don't have the ABIs being collected yet, so I don't have a place to integrate this yet
You can integrate with the existing ERC-20 parsing; it will make it easier to then later generalize from ERC-20 to [any contract whose ABI we have].
we have the ERC-20 ABI, but we don't have a piece of data telling us which contract to use which ABI for
We do have token_type
in chain.evm_tokens
. Though doing that properly will require a new analyzer that will parse events and txs post-hoc, as ABIs become available, right? Created #872 to track this.
What you can do short-term though is replace the slicing here. We'll still only heuristically decide the event is an ERC20 Transfer, but then we'll parse it with the ABI. Kinda silly, but might be a worthy early use case for the new code, and a way of testing it out gradually.
note for later: see BoundContract.UnpackLog for how to parse the topics. filter .Inputs by their .Indexed, then use abi.ParseTopics. or probably abi.ParseTopicsIntoMap for our use case |
e90ef11
to
61205ba
Compare
61205ba
to
098fdc7
Compare
b58ee98
to
340b0b2
Compare
340b0b2
to
59d0d44
Compare
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!
I'll leave it to you to decide how to show bytes
and bytes1..bytes32
to the end users. I'd go for hex, but I can see how it's a matter of opinion. I'm quite convinced the final format is mostly the domain of this code though, not the frontend.
f4550f5
to
36552bd
Compare
a88b09f
to
b91c364
Compare
oh update: uint8[] and uint8[n] don't become hex/base64. they're proper arrays of numbers |
b91c364
to
f361f74
Compare
ok bytes and bytesn are now like "0x1234" strings |
This PR has just the basics for some code to parse Ethereum ABI format things, including tx input data, tx result data, and log data. Later PRs build upon this