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

feat: raw bytecode analysing script #768

Merged

Conversation

arianejasuwienas
Copy link
Contributor

Description:
Raw bytecode analysing tool

Signed-off-by: Mariusz Jasuwienas <[email protected]>
Signed-off-by: Mariusz Jasuwienas <[email protected]>
@arianejasuwienas arianejasuwienas changed the title Raw bytecode analysing script feat: raw bytecode analysing script May 23, 2024
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Looking good.
Left some comments.

tools/custom/raw-bytecode-analyser/README.md Outdated Show resolved Hide resolved
- `--previewnet`: Optional flag to use the Previewnet Mirror Node URL.
- `--testnet`: Optional flag to use the Testnet Mirror Node URL.

If no network flag is specified, the script defaults to using the Mainnet Mirror Node URL.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer we switch this. Default to testnet and let people set a flag to point to mainnet.

tools/custom/raw-bytecode-analyser/README.md Outdated Show resolved Hide resolved
acuarica
acuarica previously approved these changes May 23, 2024
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@david-bakin-sl david-bakin-sl left a comment

Choose a reason for hiding this comment

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

Possible bug?

method_detected = True
if instruction.operand == '03' and instruction.name == 'PUSH1':
param_detected = True
if instruction.name == 'CALL':
Copy link
Member

Choose a reason for hiding this comment

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

GIven L21-22 does this ever get executed? I think things might need to get rearranged.

address_detected = True
if instruction.operand in ['0fb65bf3', '2af0c59a', 'ea83f293', 'abb54eb5']:
method_detected = True
if instruction.operand == '03' and instruction.name == 'PUSH1':
Copy link
Member

@david-bakin-sl david-bakin-sl May 23, 2024

Choose a reason for hiding this comment

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

Might be worth (for self-documenting purposes) to give a name to '03'. (I'm thinking that people will adapt this script for their own investigations and where the selectors in L25 above are kind of obviously selectors this naked '03' isn't obviously ... anything in particular ... so they might be wondering whether it's something they should change or not.) (I could go either way on the '0167' but in the context of a script named ...token_creation... it's recognizable as the system contract address.)

call_detected = False

evm_bytecode = EvmBytecode(bytecode)
disassembled_code = evm_bytecode.disassemble()
Copy link
Member

Choose a reason for hiding this comment

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

Amazing how this library makes this simple script so easy!

@david-bakin-sl
Copy link
Member

P.S. I like the way the readme carefully documents the limitations. This is a great script to easily / quickly find the contracts that might qualify, then you need to investigate more closely to really understand them. As people adapt this script for their own investigations they'll need to keep this in mind ...

…y contract id but contract evm address as well. Fixing issue with the latest commit. Exporting some of the strings to the constant in order to ensure clarity.

Signed-off-by: Mariusz Jasuwienas <[email protected]>
…y contract id but contract evm address as well. Fixing issue with the latest commit. Exporting some of the strings to the constant in order to ensure clarity.

Signed-off-by: Mariusz Jasuwienas <[email protected]>
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lgtm

@arianejasuwienas arianejasuwienas linked an issue Jun 14, 2024 that may be closed by this pull request
4 tasks
@arianejasuwienas arianejasuwienas merged commit ea72b2b into hashgraph:main Jun 14, 2024
22 of 23 checks passed
marcin-piela-ariane pushed a commit to marcin-piela-ariane/hedera-smart-contracts that referenced this pull request Jun 24, 2024
* feat: Raw bytecode analyser

Signed-off-by: Mariusz Jasuwienas <[email protected]>

* feat: Raw bytecode analyser

Signed-off-by: Mariusz Jasuwienas <[email protected]>

* feat: Adding information about the possibility to use in args not only contract id but contract evm address as well. Fixing issue with the latest commit. Exporting some of the strings to the constant in order to ensure clarity.

Signed-off-by: Mariusz Jasuwienas <[email protected]>

* feat: Adding information about the possibility to use in args not only contract id but contract evm address as well. Fixing issue with the latest commit. Exporting some of the strings to the constant in order to ensure clarity.

Signed-off-by: Mariusz Jasuwienas <[email protected]>

* feat: making sure that the description covers all possible usecases

Signed-off-by: Mariusz Jasuwienas <[email protected]>

---------

Signed-off-by: Mariusz Jasuwienas <[email protected]>
@quiet-node quiet-node added the enhancement New feature or request label Jul 12, 2024
@quiet-node quiet-node added this to the 0.8.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run Static Analysis on the hts-precompile/examples/ Contracts
5 participants