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: improve UsedStateEVMInspector by adding other op-codes that read or write #58

Open
bertmiller opened this issue Jul 13, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@bertmiller
Copy link
Member

During simulation we use an EVM inspector to get state that is used by a transaction by looking at the storage slots that a transaction reads and writes from. We could supplement this by adding other op-codes that read or write state. A few off the top of my head that we should add:

  • balance
  • selfbalance
  • create (balance and nonce is read)
  • create2 (balance and nonce is read)
  • selfdestruct (balance and nonce is read)

Do we need to support EXTCODECOPY, EXTCODEHASH, or EXTCODESIZE too? Not sure but I think so because some transactions might condition their execution on these and others might deploy contracts.

The goal would be to improve our coverage of what state is being used and how.

https://github.com/flashbots/rbuilder/blob/develop/crates/rbuilder/src/building/evm_inspector.rs

@bertmiller bertmiller added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Jul 13, 2024
@bertmiller bertmiller changed the title feat: improve UsedStateEVMInspector by adding other op-codes that read feat: improve UsedStateEVMInspector by adding other op-codes that read or write Jul 13, 2024
@nikoseven
Copy link
Contributor

nikoseven commented Jul 30, 2024

I'd like to work on this.
I'm considering adding three more fields to UsedStateTrace. Does that sound good?

pub struct UsedStateTrace {
     ...
    // account read set
    pub balance_read_set: HashMap<Address, U256>,
    // list for contract create/destruct
    pub created_contract_addr_list: Vec<Address>,
    pub destructed_contract_addr_list: Vec<Address>,
}

@ZanCorDX
Copy link
Contributor

To have all the balance information we might need I believe it would be better to have something like this:

   // first balance account read
    pub balance_read_set: HashMap<Address, U256>,
    // last balance account written
    pub balance_write_set: HashMap<Address, U256>,

What do you think?

@ZanCorDX
Copy link
Contributor

The names we already had were not the best, read_set/write_set are not very meaningful.
It might be nice to correct them (if possible in another PR) to something like read_slot_values/written_slot_values (or read_slots/written_slots?) so your fields can be more like read_balances/written_balances and created_contracts/destructed_contracts.
What do you think?

ZanCorDX pushed a commit that referenced this issue Aug 8, 2024
## 📝 Summary
Rename `read_set/write_set` to `read_slot_values/written_slot_values`
## 💡 Motivation and Context

As discussed in #58, we
renamed the fields to make it more descriptive and clear with the coming
fields we are going to add.

---

## ✅ I have completed the following steps:

* [x] Run `make lint`
* [x] Run `make test`
* [ ] Added tests (if applicable)
dvush pushed a commit that referenced this issue Sep 20, 2024
## 📝 Summary
Added support for SELFBALANCE, BALANCE, CALL, CREATE, and SELFDESTRUCT
op-codes in UsedStateEVMInspector.
New test cases for evm inspector have been added.

## 💡 Motivation and Context
Related discussion in #58.
For opcodes like EXTCODECOPY, EXTCODEHASH, and EXTCODESIZE, I thought we
could add them in the future with a concrete use case, so that we can
properly design the datastruct for it.

---

## ✅ I have completed the following steps:

* [x] Run `make lint`
* [x] Run `make test`
* [x] Added tests (if applicable)
astarinmymind pushed a commit that referenced this issue Oct 3, 2024
## 📝 Summary
Added support for SELFBALANCE, BALANCE, CALL, CREATE, and SELFDESTRUCT
op-codes in UsedStateEVMInspector.
New test cases for evm inspector have been added.

## 💡 Motivation and Context
Related discussion in #58.
For opcodes like EXTCODECOPY, EXTCODEHASH, and EXTCODESIZE, I thought we
could add them in the future with a concrete use case, so that we can
properly design the datastruct for it.

---

## ✅ I have completed the following steps:

* [x] Run `make lint`
* [x] Run `make test`
* [x] Added tests (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants