Skip to content

fix withdrawal bal index tracking#28

Closed
nerolation wants to merge 1 commit intofselmo:feat/amsterdam-fork-and-block-access-listsfrom
nerolation:fix-wd
Closed

fix withdrawal bal index tracking#28
nerolation wants to merge 1 commit intofselmo:feat/amsterdam-fork-and-block-access-listsfrom
nerolation:fix-wd

Conversation

@nerolation
Copy link
Collaborator

What was wrong?

I'm not 100% sure how this could have occurred but this should make it bullet-proof.

Related to #23

@cr-gpt
Copy link

cr-gpt bot commented Oct 29, 2025

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

@nerolation
Copy link
Collaborator Author

nerolation commented Oct 29, 2025

Could it be an issue with the test?

I cannot spot the issue in the specs right now

)

for i, tx in enumerate(map(decode_transaction, transactions)):
decoded_txs = list(map(decode_transaction, transactions))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mapping an array doesn’t change its length each element produces exactly one result, even if decoding fails.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right, but I cannot see the issue here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I placed an exit(1) after this code and the code path is not getting hit, so the bug is elsewhere. I noticed that amsterdam doesn't call self.fork.process_unchecked_system_transaction like prague and cancun, maybe that has something to do with this?

if self.fork.is_after_fork("amsterdam"):
self.fork.set_block_access_index(
block_env.state.change_tracker, Uint(0)
)
if self.fork.is_after_fork("prague"):
self.fork.process_unchecked_system_transaction(
block_env=block_env,
target_address=self.fork.HISTORY_STORAGE_ADDRESS,
data=block_env.block_hashes[-1], # The parent hash
)
if self.fork.is_after_fork("cancun"):
self.fork.process_unchecked_system_transaction(
block_env=block_env,
target_address=self.fork.BEACON_ROOTS_ADDRESS,
data=block_env.parent_beacon_block_root,
)

Copy link
Owner

Choose a reason for hiding this comment

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

I noticed that amsterdam doesn't call self.fork.process_unchecked_system_transaction like prague and cancun, maybe that has something to do with this?

Amsterdam is after both Prague and Cancun, all of these lines should be called by Amsterdam.

Copy link
Owner

@fselmo fselmo Oct 29, 2025

Choose a reason for hiding this comment

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

The state of things here should be:

  • Amsterdam first sets the block access index before any of the system txs
  • Prague line should be hit processing history storage contract call
  • Cancun line is hit processing beacon roots contract call

Copy link
Owner

@fselmo fselmo Oct 29, 2025

Choose a reason for hiding this comment

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

Where is this being discussed? This could be an issue with the rebase after the weld, tbh. I will try filling and see if I notice anything strange but please provide more context here if you can.

Copy link
Owner

@fselmo fselmo Oct 29, 2025

Choose a reason for hiding this comment

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

Ah, #26 👍🏼

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we're missing something else, an exit(1) is not hitting this line though:

post_execution_index = ulen(transactions) + Uint(1)

If it's not hitting that line then the transaction is failing, no? Without looking much into it if we never process_transaction appropriately is the only case we wouldn't hit that line? 👀

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah super weird the transaction goes through and we get a BAL mismatch error:

E   execution_testing.test_types.block_access_list.exceptions.BlockAccessListValidationError: Balance change (2, 10000000000) not found or not in correct order. Actual changes: [(1, 10000000000)]

Copy link
Owner

@fselmo fselmo Oct 29, 2025

Choose a reason for hiding this comment

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

I found the culprit. I fixed it in the tests PR #23 @nerolation @raxhvl here: 2617d30. Sorry, seemed easier since I was in there already running the tests and now we can modify both spec and tests in the same repo / PR 🔥

Super quirky / tricky to find haha

@fselmo
Copy link
Owner

fselmo commented Oct 29, 2025

Closing in favor of fix in #23 here (2617d30)

@fselmo fselmo closed this Oct 29, 2025
fselmo pushed a commit that referenced this pull request Jan 6, 2026
* chore: add grammar check automation files

* docs: fix grammar in consume plugin files (#20)

Fix grammar issues in consume plugin docstrings and comments:
- "A dictionary caches" -> "A dictionary that caches" (missing "that")
- "Definers" -> "Defines" (typo)
- "Un-expected" -> "Unexpected" (incorrect hyphenation)
- "older version" -> "older versions" (subject-verb agreement)

Co-authored-by: Claude <noreply@anthropic.com>

* chore(docs): fix grammar in filler plugin docstrings (#21)

- Fix subject-verb agreement: "The forks is" -> "The fork is"
- Fix preposition: "value to each" -> "value for each"
- Fix sentence structure: "(which) ... occurs" -> ", which ... that occurs"

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in execute plugin files (#22)

- Fix "using in live networks" -> "on live networks" (execute.py:2)
- Fix missing spaces between concatenated help strings in execute.py
- Fix "using for test filling" -> "used for test filling" (pre_alloc.py:1)
- Fix missing spaces in concatenated help strings in remote.py

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in forks, shared, and help plugin files (#23)

Fixed 9 grammar issues:
- forks/__init__.py: Added missing "and" in docstring
- forks/forks.py: Fixed "ran" -> "run" (passive voice)
- forks/forks.py: Removed space in URL fragment
- forks/forks.py: Fixed double "any any" -> "any"
- test_bad_command_line_options.py: Fixed article/noun agreement
- test_bad_validity_markers.py: Fixed article/noun agreement
- shared/helpers.py: Fixed verb tense for parallel structure
- shared/execute_fill.py: Added missing article "A"
- help/help.py: Fixed double article "the a" -> "a"


Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in pytest_commands/plugins root files (#24)

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in cli/pytest_commands root files (#25)

Remove extra space after newline in watcher.py watch mode message.

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in cli/gentest and cli/fuzzer_bridge (#26)

- Fix article "An provider" → "A provider" (test_context_providers.py)
- Fix subject-verb agreement "generates" → "generate" (test_providers.py)
- Add missing preposition "for" in "A request manager for Ethereum RPC calls" (request_manager.py)

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in cli/eest, cli/input, and cli/fillerconvert (#27)

* docs: fix grammar in cli/eest, cli/input, and cli/fillerconvert

Fix missing space in string concatenation in quotes.py where
"that" and "experience" were joined without a space.

Flag one low-confidence issue in verify_filled.py for manual
review regarding informal verb usage.

* docs: apply manual grammar fix in verify_filled.py

Improve docstring clarity by:
- Adding missing articles ("the original", "the refilled test")
- Replacing informal "Regex" verb with "Extract"
- Reformatting for better readability

---------

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in packages/testing/src/execution_testing/cli/*.py (#28)

- gen_index.py: Add missing space between sentences in help text
- modify_static_test_gas_limits.py: Remove duplicate word "files files"
- tox_helpers.py: Remove extra space in error message
- show_pre_alloc_group_stats.py: Fix capitalization mid-sentence

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in packages/testing/src/execution_testing/cli/tests (#29)

- Fix "allows to actually fill" → "allows actually filling" (missing verb form)
- Fix "uses the a temporary" → "uses a temporary" (double article)

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in test_types module (#30)

- Fix double space in helpers.py docstring
- Complete incomplete sentence in trie.py module docstring

Co-authored-by: Claude <noreply@anthropic.com>

* docs: grammar check specs module (round 12) (#31)

Reviewed 28 files in packages/testing/src/execution_testing/specs/**/*.py
for grammar issues.

Fixed 1 grammar issue:
- blockchain.py:167-168 - fixed sentence fragment "encoding. included"

Co-authored-by: Claude <noreply@anthropic.com>

* docs: grammar check round 13 (fixtures + client_clis) (#32)

Reviewed packages/testing/src/execution_testing/fixtures/**/*.py (16 files)
- No grammar issues found

Reviewed packages/testing/src/execution_testing/client_clis/**/*.py (20 files)
Fixed 4 grammar issues:
- ethereum_cli.py:158 - Added missing space after "0!"
- evmone.py:207 - "an select" -> "and select"
- nethermind.py:209 - "an select" -> "and select"
- geth.py:383 - "an select" -> "and select"

Flagged 1 item for manual review:
- clis/__init__.py:1 - "CL interfaces" may be typo for "CLI interfaces"

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in base_types module (#33)

- Fix "it can added" -> "it can be added" in mixins.py
- Fix "and returns" -> "and return" for consistent imperative form in composite_types.py
- Fix "an hexadecimal numbers" -> "hexadecimal numbers" in base_types.py

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in forks module (ethereum#34)

base_fork.py:
- precompiles(): "Return list precompiles" -> "Return list of precompiles"
- system_contracts(): "Return list system-contracts" -> "Return list of system contracts"

forks/forks/forks.py:
- Standardize "pre-compiles" -> "precompiles" (no hyphen) in:
  - Frontier.precompiles()
  - Homestead.precompiles()
  - Byzantium.precompiles()
  - Istanbul.precompiles()
  - Cancun.precompiles()
  - Prague.precompiles()
  - Osaka.precompiles()
- Standardize "system-contracts" -> "system contracts" in Frontier.system_contracts()
- Fix subject-verb agreement: "pre-compile for BLS operations are" -> "precompiles for BLS operations are"
- Fix max_initcode_size(): merge standalone string into docstring with proper formatting
- Add missing article "a" before "precompile for" in Istanbul, Cancun, and Osaka

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in tools and exceptions modules (ethereum#35)

Fix typos and grammar issues:
- Fix "defniitions" -> "definitions" in engine_api.py
- Fix "it's" -> "its" (possessive) in eof.py and exceptions.py
- Fix "an stack" -> "a stack" for underflow/overflow in eof.py and exceptions.py

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in vm, rpc, and config modules (ethereum#36)

Remove duplicate text in error message in opcodes.py

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix grammar in execution_testing core modules (logging, execution, checklists, benchmark) and root files (ethereum#37)

Fixed "were request" -> "were requested" in blob_transaction.py comment.

Co-authored-by: Claude <noreply@anthropic.com>

* docs: fix docstring in client_clis module

Corrected terminology from "CL interfaces" to "EL clients" - these are
Execution Layer client implementations, not Consensus Layer.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* docs: fix grammar in exceptions module

- block.py: "is calculated incorrect" → "is calculated incorrectly"
- transaction.py: "is exceeds" → "exceeds" (removed double verb)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* docs: fix punctuation in checklists module

Changed period to colon in DataPortionVariables docstring to properly
introduce the list of test cases (Top, Bottom, Middle).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* chore: remove files used for llm automation

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants