Skip to content

endorser/execution: cover DualStateDB to 98% with dual-side assertions - #345

Merged
arner merged 3 commits into
hyperledger:mainfrom
Storm1289:endorser-execution-coverage-320
Sep 1, 2026
Merged

endorser/execution: cover DualStateDB to 98% with dual-side assertions#345
arner merged 3 commits into
hyperledger:mainfrom
Storm1289:endorser-execution-coverage-320

Conversation

@Storm1289

@Storm1289 Storm1289 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #320.

Adds endorser/execution/dual_statedb_test.go. The tests build both a go-ethereum in-memory state.StateDB and the endorser SnapshotDB, wrap them in a DualStateDB, and assert that every mutation is reflected consistently on both sides.

Also adds four focused tests for EVMEngine.BalanceAt, StorageAt, CodeAt, and NonceAt, covering the fresh-snapshot read path.

Coverage delta

File Before After
dual_statedb.go (issue's primary target) 0% 98.3%
statedb.go ~78% 83.3%
Package total 22.3% 47.4%

What's tested

All method categories from the issue are covered:

  • Accounts: CreateAccount, CreateContract, Exist, Empty, Touch
  • Balances/nonces: AddBalance, SubBalance, SetNonce
  • Code: SetCode, GetCodeSize, GetCodeHash
  • Storage: SetState, GetStateAndCommittedState, GetStorageRoot, transient state
  • Refunds
  • Self-destruct
  • Access list, including Prepare precompile seeding
  • Snapshot / revert round-trip
  • Logs
  • Finalise
  • Result
  • Logs
  • IsNewContract
  • LogsForBurnAccounts
  • Witness
  • AccessEvents

Open question for the maintainer

The package-level coverage is capped by the two *_logger.go files (eth_statedb_logger.go and statedb_logger.go), which the issue explicitly lists as out of scope. Together they are ~770 LOC of pure logging pass-throughs (~22% of the package by size), gated behind the DebugLogs config, and both remain at ~0%.

Two ways to reconcile the 70% target while keeping those files out of scope:

  1. Add endorser/execution/*_logger.go to the codecov.yml ignore list, matching the existing **/*.pb.go / **/*.sql.go pattern for non-review-worthy code. In-scope coverage would then be represented accurately.
  2. Interpret the 70% target as applying to the in-scope files, where coverage is already ~90%.

I didn't touch codecov.yml in this PR. Happy to add the ignore lines in a follow-up commit if you prefer option 1.

Out of scope

  • eth_statedb_logger.go / statedb_logger.go - debug wrappers, config-gated
  • Integration-level EVM execution - covered by the integration suite

Test plan

  • go build ./...
  • go test -race -count=1 ./endorser/execution/...
  • go test -cover ./endorser/execution/...47.4% package, 98.3% on dual_statedb.go
  • CI Quick Tests

- New dual_statedb_test.go builds both a go-ethereum StateDB and the
  endorser SnapshotDB, wraps them in a DualStateDB, and asserts each
  mutation is reflected on both sides.
- Covers accounts, balances/nonces, code, storage (incl. transient and
  committed), refunds, self-destruct, access list, Prepare, snapshot /
  revert, logs, Finalise, Result, and pass-through accessors.
- Adds EVMEngine read-helper tests (BalanceAt / StorageAt / CodeAt /
  NonceAt) for the fresh-snapshot path.
- dual_statedb.go coverage: 0% -> 98.3%. Package: 22.3% -> 47.4%.
  Remainder lives in *_logger.go which the issue lists as out of scope.

Signed-off-by: Storm1289 <divakarsharma2934@gmail.com>
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.62%. Comparing base (09ef2a0) to head (eaf056c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
+ Coverage   34.07%   36.62%   +2.54%     
==========================================
  Files          82       82              
  Lines       11652    11652              
==========================================
+ Hits         3970     4267     +297     
+ Misses       7347     7042     -305     
- Partials      335      343       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

🎉 OZ Hardhat Compatibility — 11 tests fixed!

Stale baseline entries (11) — remove these

  • AccessManager admin operations subject to a delay #setRoleAdmin restrictions when the call does not come from the manager (msg.sender != manager) when the function requires the caller to be granted with a role other than PUBLIC_ROLE when the required role is granted to the caller when role granting is not delayed when caller has no execution delay succeeds via execute
  • AccessManager admin operations subject to a delay #setRoleGuardian restrictions when the call does not come from the manager (msg.sender != manager) when the function requires the caller to be granted with a role other than PUBLIC_ROLE when the required role is granted to the caller when role granting is not delayed when caller has no execution delay succeeds via execute
  • AccessManager admin operations subject to a delay #setGrantDelay restrictions when the call does not come from the manager (msg.sender != manager) when the function requires the caller to be granted with a role other than PUBLIC_ROLE when the required role is granted to the caller when role granting is not delayed when caller has no execution delay succeeds via execute
  • AccessManager admin operations subject to a delay #setTargetAdminDelay restrictions when the call does not come from the manager (msg.sender != manager) when the function requires the caller to be granted with a role other than PUBLIC_ROLE when the required role is granted to the caller when role granting is not delayed when caller has no execution delay succeeds via execute
  • AccessManager admin operations not subject to a delay #setTargetFunctionRole restrictions when the call does not come from the manager (msg.sender != manager) when the function requires the caller to be granted with a role other than PUBLIC_ROLE when the required role is granted to the caller when role granting is not delayed when caller has no execution delay succeeds via execute
  • AccessManager admin operations not subject to a delay role admin operations #grantRole restrictions when the call does not come from the manager (msg.sender != manager) when the function requires the caller to be granted with a role other than PUBLIC_ROLE when the required role is granted to the caller when role granting is not delayed when caller has no execution delay succeeds via execute
  • UUPSUpgradeable upgrade to upgradeable implementation with call
  • ERC721Wrapper onERC721Received mints a token to from
  • Create2 deploy deploys a contract without constructor
  • GovernorProposalGuardian using $ERC20Votes "before each" hook for "deployment check"
  • GovernorProposalGuardian using $ERC20VotesTimestampMock "before each" hook for "deployment check"
Full breakdown — 5519 passed, 874 failed, 1 skipped (6394 total, 86.3% passing)

By suite

  • access: 408/603 passing (68%)
  • crosschain: 44/48 passing (92%)
  • finance: 8/16 passing (50%)
  • governance: 243/532 passing (46%)
  • metatx: 26/32 passing (81%)
  • proxy: 287/320 passing (90%)
  • token: 1800/1882 passing (96%)
  • utils: 2703/2960 passing (91%)

Expected failures by cause (608)

  • max-code-size: 212
  • execution reverted: 129
  • hardhat-time-rpc: 93
  • fixed-timestamp: 35
  • subcall-effect-lost: 31
  • fixed-block-number: 23
  • hardhat_setStorageAt: 17
  • gas required exceeds allowance (10000000): 12
  • eth_getProof: 6
  • no private key available for address 0x8B148f5D02D560f1f46438656a85C2d3cC143390: 5
  • Expected transaction to be reverted with custom error 'FailedCall', but it reverted with custom error 'TimelockUnexpectedOperationState': 4
  • Expected transaction to be reverted with custom error 'InvalidAccountNonce', but it reverted with custom error 'ERC3009InvalidAuthorizationTime': 4
  • evm_setAutomine: 3
  • execfailure-dropped-not-committed: 3
  • no private key available for address 0xB40Dc4D4FE972F7C3BD3979d0B5E9582073E6E7E: 3
  • out of gas: 3
  • transaction already pending: 3
  • Expected transaction to be reverted with custom error 'FailedCall', but it didn't revert: 2
  • estimate-gas-allowance-not-revert: 2
  • hardhat-block-rpc: 2
  • hardhat_impersonateAccount: 2
  • hardhat_setBalance: 2
  • no private key available for address 0x09120eAED8e4cD86D85a616680151DAA653880F2: 2
  • no private key available for address 0x0B306BF915C4d645ff596e518fAf3F9669b97016: 2
  • Expected transaction to be reverted with custom error 'ERC3009InvalidAuthorizationTime', but it didn't revert: 1
  • Expected transaction to be reverted with custom error 'TimelockUnexecutedPredecessor', but it reverted with custom error 'TimelockUnexpectedOperationState': 1
  • Expected transaction to be reverted with panic code 0x01 (Assertion error), but it reverted with a custom error: 1
  • could not decode result data (value="0x", info={ "method": "getValue", "signature": "getValue()" }, code=BAD_DATA, version=6.16.0): 1
  • expected '0xaeee8799c04cdd8ea4d908c7d22d99ea63c…' to equal '0x7f6f6e29c15580dc65a7ad00b0ca049e17a…': 1
  • expected 36000 to equal 4919.: 1
  • expected 4919 to equal 0.: 1
  • no private key available for address 0xc6e7DF5E7b4f2A278906862b61205850344D4e7d: 1

Lock in the win — run go run ./cmd/baseline update --suite oz-hardhat and commit the updated baseline to remove these.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

Perf Smoke — CI baseline (lower-bound against test committer)

Metric Value
Throughput 520.90 tx/s
Invalid rate 0.000000
Conflict rate 0.000000

Workflow run

@Storm1289
Storm1289 marked this pull request as ready for review August 25, 2026 10:29

@arner arner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Also functions as documentation on which features we do implement and what we skip.

@Storm1289
Storm1289 requested a review from arner August 31, 2026 12:58

@arner arner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@arner
arner merged commit f95dd8c into hyperledger:main Sep 1, 2026
8 checks passed
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.

Increase endorser/execution test coverage to 70%

2 participants