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.

Sign up for GitHub

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

Jump to bottom

feat: Add other state used to ConflictFinder #226

Open
bertmiller opened this issue Oct 24, 2024 · 2 comments
Open

feat: Add other state used to ConflictFinder #226

bertmiller opened this issue Oct 24, 2024 · 2 comments
Labels
A-builder Issues related to the core block building protocol enhancement New feature or request help wanted Extra attention is needed

Comments

@bertmiller
Copy link
Member

bertmiller commented Oct 24, 2024 •
edited
Loading

The job of the conflict finder is to identify potential conflicting orders. Currently it does this by looking at storage read and write overlaps. We need to add the other possible state access patterns too. Luckily we already have these in the UsedStateTrace but they need to be reflected in the ConflictFinder.

Also, we should add nonces to this.
@bertmiller bertmiller added enhancement New feature or request help wanted Extra attention is needed A-builder Issues related to the core block building protocol labels Oct 24, 2024
@grasphoper
Copy link
Contributor

grasphoper commented Nov 2, 2024 •
edited
Loading

Hi @bertmiller, can I take this?

@grasphoper
Copy link
Contributor

grasphoper commented Nov 3, 2024 •
edited
Loading

I've taken a look at the code and other possible conflicts to add. They are:

  • read_balances can conflict with sent_amount, received_amount
  • read_slot, write_slot can conflict with destructed_contracts
  • destructed_contracts can conflict with each other
  • created_contracts can conflict with each other

Do we want to add all of these? And did you have any other combinations in mind?

As for the nonces, they seem to be partially taken care of already, as they're recorded as read_slot + write_slot for EOA at 0th slot here. There's a corner case though: right now, if we have 2 TXs that are using different nonces for the same EOA, we deem them "conflicting", which should not be the case. Not obvious how to work around this yet (other than changing how the nonce usage is recorded into the UsedStateTrace)

@grasphoper grasphoper mentioned this issue Nov 5, 2024
Merged
3 tasks
ZanCorDX pushed a commit that referenced this issue Dec 12, 2024
@grasphoper
Add more conflict types to ConflictFinder (#238)
16290b6 
## 📝 Summary

Add more conflict types to `ConflictFinder` as per
[#226](#226)

## A brief overview of the changes:
- I added multiple new mapping entries to `ConflictFinder`, namely 
```rust     
    group_balance_reads: HashMap<Address, Vec<usize>>,
    group_balance_writes: HashMap<Address, Vec<usize>>,
    group_contract_creations: HashMap<Address, Vec<usize>>,
    group_contract_destructions: HashMap<Address, Vec<usize>>,
```
- Added conflicts that can be related to those fields
- Added `combine_groups` - a utility fn to reuse code
- Added `ConflictFinder.add_group_to_index` and
`ConflictFinder.remove_group_from_index` to reuse code and improve
readability; `index` here means search index, or different member
mappings of `ConflictFinder`

---

## ✅ I have completed the following steps:

* [x] Run `make lint`
* [X] Run `make test`
* [X] Added tests (if applicable)

---------

Co-authored-by: dev <dev@Ihors-MacBook-Air.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-builder Issues related to the core block building protocol enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bertmiller @grasphoper