Skip to content

fix: bal binary search cases#3139

Merged
rakita merged 3 commits intobluealloy:rakita/balfrom
dajuguan:po_bal_binary_fix
Nov 3, 2025
Merged

fix: bal binary search cases#3139
rakita merged 3 commits intobluealloy:rakita/balfrom
dajuguan:po_bal_binary_fix

Conversation

@dajuguan
Copy link

@dajuguan dajuguan commented Nov 3, 2025

This PR fixes a bug in the BAL lookup method, where the binary search logic could return incorrect results in certain edge cases.

  • When BalWrites length > 5:
    • case 1: The returned index should be decremented by 1 to align with BAL lookup semantics.
    • case 2: When bal_index is 0, the binary search function should return None.
  • Handles the case where the searched bal_index does not exist in the write list correctly.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #3139 will improve performances by 3.42%

Comparing dajuguan:po_bal_binary_fix (914e745) with rakita/bal (d9d8896)1

Summary

⚡ 1 improvement
✅ 172 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
snailtracer-inspect 208.5 ms 201.6 ms +3.42%

Footnotes

  1. No successful run was found on rakita/bal (efe3be8) during the generation of this report, so 0ce3c96 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm, good bug found.

Have slighlty simplified the code

@rakita rakita merged commit 180f9ff into bluealloy:rakita/bal Nov 3, 2025
2 checks passed
rakita added a commit that referenced this pull request Dec 18, 2025
* BAL

* WIP

* bal wip

* bal followup

* Database::bal and Bal IndexMap for accounts

* bal builder and integration with databases

* chore: bump eest tests v5.3.0

* bump bal for caller/beneficiary

* bal builder from state on commit, alloy included

* cleanup

* bal integration in btests

* wip sys call

* fix few bugs, propagate error

* remove bal panic from btest

* error handling

* cleanup bal tests

* skip touching beneficiary if reward is 0

* handle local selfdestruct

* feat: dont load access list immediatly

* nits fmt

* bump output as accounts now have original account info

* BalDatabase

* nit, rm clone

* bump tests, add missing imports, cleanup

* reause indexmap from alloy with default hasher

* typos

* add missing serde propagation

* dont skip test

* Create BalState and add it inside State so that we dont need to use BalDatabase

* nits, and deserialization for Account without original info

* propagate feature

* fix: add bal_builder.commit to state, small cleanup

* fix: bal binary search cases (#3139)

* fix: bal binary search cases

* nit(test): generalize BAL binary search test for any threshold

* code cleanup

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>

* compile tests

* Rename BalDatabaseError to EvmDatabaseError

* throw error if bal exist but account/storage not

* rename storage_id to account_id

* rm println

* use alloy main, clippy/typo fixes

* ark the bytecode

* typo

* add statis default for Bytecode

* use oncelock

* try with oncelock

* box original acc info
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 21, 2026
* BAL

* WIP

* bal wip

* bal followup

* Database::bal and Bal IndexMap for accounts

* bal builder and integration with databases

* chore: bump eest tests v5.3.0

* bump bal for caller/beneficiary

* bal builder from state on commit, alloy included

* cleanup

* bal integration in btests

* wip sys call

* fix few bugs, propagate error

* remove bal panic from btest

* error handling

* cleanup bal tests

* skip touching beneficiary if reward is 0

* handle local selfdestruct

* feat: dont load access list immediatly

* nits fmt

* bump output as accounts now have original account info

* BalDatabase

* nit, rm clone

* bump tests, add missing imports, cleanup

* reause indexmap from alloy with default hasher

* typos

* add missing serde propagation

* dont skip test

* Create BalState and add it inside State so that we dont need to use BalDatabase

* nits, and deserialization for Account without original info

* propagate feature

* fix: add bal_builder.commit to state, small cleanup

* fix: bal binary search cases (bluealloy/revm#3139)

* fix: bal binary search cases

* nit(test): generalize BAL binary search test for any threshold

* code cleanup

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>

* compile tests

* Rename BalDatabaseError to EvmDatabaseError

* throw error if bal exist but account/storage not

* rename storage_id to account_id

* rm println

* use alloy main, clippy/typo fixes

* ark the bytecode

* typo

* add statis default for Bytecode

* use oncelock

* try with oncelock

* box original acc info
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.

2 participants