Skip to content

Conversation

@ARR4N
Copy link
Collaborator

@ARR4N ARR4N commented Nov 13, 2025

Introduces test-only builders of blocks and transactions. Maintaining block lineage and per-key nonces is a common pattern that I noticed in all of my SAE development, so these abstract them.

Testing is performed against a libevm/core.BlockChain as the gold standard, as these helpers will be key to testing the saexec.Executor to be introduced later.

@ARR4N ARR4N self-assigned this Nov 13, 2025
@ARR4N ARR4N marked this pull request as ready for review November 13, 2025 16:43
@ARR4N ARR4N requested a review from a team November 13, 2025 16:45
@ARR4N ARR4N requested review from a team and michaelkaplan13 November 14, 2025 09:13
Copy link
Contributor

@alarso16 alarso16 left a comment

Choose a reason for hiding this comment

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

Actual mechanics look good to me, testing is super clear too

}
}
b := build.NewBlock(t, txs, ModifyHeader(func(h *types.Header) {
h.GasLimit = 100e6
Copy link
Contributor

Choose a reason for hiding this comment

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

The gas limit is an annoying thing to have to specify every time, what if you just made it "sufficiently large" so that it won't cause problems for you rest?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is only necessary when using a core.StateProcessor(), which will never be the case outside of this one single test as everything else will use the SAE executor.

sdb, err := state.New(gen.SettledStateRoot(), state.NewDatabase(db), nil)
require.NoError(t, err, "state.New(genesis.SettledStateRoot())")
for i, addr := range wallet.Addresses() {
want := new(uint256.Int).SetAllOne()
Copy link
Contributor

Choose a reason for hiding this comment

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

You could move this outside the loop, since it's shared

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I considered that, and in production I would, but I prioritised reducing cognitive load by co-locating the expected value with the assertion.

ARR4N and others added 3 commits November 14, 2025 15:08
Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com>
Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>
@ARR4N ARR4N requested a review from alarso16 November 14, 2025 15:29
@ARR4N ARR4N merged commit 1318968 into main Nov 17, 2025
11 checks passed
@ARR4N ARR4N deleted the arr4n/block-and-tx-builders branch November 17, 2025 10:28
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.

6 participants