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

[WIP] Implement getCode cheatcode #593

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

0xZRA
Copy link
Contributor

@0xZRA 0xZRA commented Mar 10, 2025

No description provided.

@0xZRA
Copy link
Contributor Author

0xZRA commented Mar 10, 2025

This is still WIP
@anishnaik Looking for early feedback. I settled on ContractBytecodes or CompiledContracts to avoid adding the logic for opening the file + looking for the bytecode in the handler (but still be able to connect compiled contract with the tracer). Using CompiledContracts will provide contract context access for future cheatcode implementations but I don't know if it's truly needed at this time. I additionally tried the following strategies but didn't get successful results:

  • Added Fuzzer field to TestChain struct:
    • Set the reference when creating the chain
    • Access Fuzzer's contract definitions through this reference
    • Issue: Creates import cycle between packages (test_chain.go <-> fuzzer.go)
  • Use fuzzer context passed to TestChain
    • Store and retrieve Fuzzer through context
    • Issue: probably not best use of the context

On a separate note, fuzzing.fuzzer_test.TestCheatCodes fails on the master branch on my machine

@0xZRA 0xZRA marked this pull request as ready for review March 10, 2025 21:29
@anishnaik
Copy link
Collaborator

Hey @0xZRA sorry for the delay - will provide feedback today :)


require(deployedHash == referenceHash, "Retrieved bytecode doesn't match reference");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sure to add a newline here

@anishnaik
Copy link
Collaborator

@0xZRA before reviewing the PR i want to make sure we satisfy the issue. It is possible that the creator of the issue does in fact want the versioning support as well. If they do then you may need to add support for that as well.

@0xZRA
Copy link
Contributor Author

0xZRA commented Mar 20, 2025

@0xZRA before reviewing the PR i want to make sure we satisfy the issue. It is possible that the creator of the issue does in fact want the versioning support as well. If they do then you may need to add support for that as well.

@anishnaik For sure, makes sense to me

Copy link
Collaborator

@anishnaik anishnaik left a comment

Choose a reason for hiding this comment

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

No major feedback outside of the optimization I mentioned :) Appreciate all the work!

@@ -94,6 +95,10 @@ type TestChain struct {
// stateFactory used to construct state databases from db/root. Abstracts away the backing RPC when running in
// fork mode.
stateFactory state.MedusaStateFactory

ContractBytecodes map[string][]byte
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 having a pointer reference to the CompiledContracts over the bytecodes since it is more memory efficient and we can access the same information.

@0xZRA
Copy link
Contributor Author

0xZRA commented Mar 21, 2025

No major feedback outside of the optimization I mentioned :) Appreciate all the work!

I should be able to wrap this up within next few days

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.

None yet

2 participants