Skip to content

fix(cmd/devp2p): price fixture txs at the chain gas floor, close #3062 - #3065

Open
gzliudan wants to merge 1 commit into
XinFinOrg:dev-upgradefrom
gzliudan:fix-ethtest-fixture-gas-price
Open

gzliudan wants to merge 1 commit into
XinFinOrg:dev-upgradefrom
gzliudan:fix-ethtest-fixture-gas-price

Conversation

@gzliudan

@gzliudan gzliudan commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

 generateTxs(numTxs)                       LiveTxRequest_Xdc166 matrix branch
-  gasPrice = big.NewInt(1)                gasPrice = big.NewInt(DefaultMinGasPrice*2500)
+  gasPrice = s.chainGasPriceFloor()       gasPrice = s.chainGasPriceFloor()
+  assert each signed tx >= floor
chainGasPriceFloor()
  pending = chain.Head().Number() + 1
  return params.GetMinGasPrice(pending, chain.config)
         |
         +-- 1x     = 250,000,000 wei      (fixture without gas forks)
         +-- 50x    = 12,500,000,000 wei
         +-- 2500x  = 625,000,000,000 wei (testdata/xdc, gas2500xBlock=10)

generateTxs priced every fixture batch at 1 wei. The XDC txpool validates against params.GetMinGasPrice(pendingNumber, config) (core/txpool/validation.go:307), so on the hydrated gas2500x fixture the whole 2000-tx batch was rejected with under min gas price. The live-devnet branch of the same test had a hardcoded DefaultMinGasPrice*2500 multiplier; the fixture branch was never adapted. Both branches now derive the price from the chain's own fork schedule at the pending block height, the exact source the node uses, and generateTxs refuses to return a batch priced below that floor. The upstream 2000-tx calibration is unchanged in this PR; changing it is tracked in #3067 (see the residual note below).

Files: cmd/devp2p/internal/ethtest/transaction_suite.go (helper, pricing, assertion, matrix branch) and transaction_suite_test.go (two-tier regression + pending-height boundary case). Suite only: no node, txpool, protocol, config, or dependency changes. No generated-code changes.

Evidence

  • Before: generateTxs used big.NewInt(1); the frozen devnet node logged under min gas price for the fixture batch and LargeTxRequest_Xdc166 reported missing 2000 txs.
  • After (chain-level A/B, silent-phase devnet, two independent rounds): with a devp2p built from dev-upgrade, each round logs under min gas price = 1; with a devp2p built from this branch, under min gas price = 0 in both rounds. The price-rejection path is gone. In matrix mode (48 txs) both binaries return ok 1 LargeTxRequest_Xdc166, so unifying the two branches does not regress the live-devnet path.
  • Unit tests: TestGenerateTxsClearsChainGasFloor covers the 1x (testdata) and 2500x (testdata/xdc) fixtures and asserts every tx equals the derived floor, with sequential nonces and matching hashes; TestChainGasPriceFloorUsesPendingNumber pins the head+1 rule at a fork boundary (601 -> 2500x, 602 -> 1x). go test ./cmd/devp2p/internal/ethtest/... -count=1 = ok 18.184s.
  • Mutations (3/3 KILLED): reverting floor to big.NewInt(1), dropping + common.Big1, and pinning the 2500x tier while ignoring the chain config each fail the new tests; probes were removed and the restored tree passes.
  • Full per-commit chain: 1/1 PASS, make all, make generate (No stale files detected.), make tidy (go mod tidy -diff clean), clean-tree gate, make lint (0 issues.), go vet ./..., make test all green. gofmt -s/goimports/git diff --check clean.
  • Residual, not fixed here (please note before merging): the fixture-mode 2000-tx LargeTxRequest_Xdc166 still reports not ok on the live devnet, but the node now rejects with nonce too high (~255 per round), not under min gas price — the pricing gate from cmd/devp2p: fixture-mode generateTxs prices at 1 wei, below the hydrated pool floor #3062 is gone. This is a separate suite defect. The corrected mechanism: nonce too high is raised during admission at core/txpool/validation.go:234 (PendingNonce + common.LimitThresholdNonceInQueue=10 < tx.Nonce()) and :241 (gapped nonce), so those txs never enter the queue; AccountQueue=64 (core/txpool/legacypool/queue.go:199) only trims the few that do. The real blockers are the node's maxTxRetrievals=256 batched requests (eth/fetcher/tx_fetcher.go:48, with f.announces iterated in map order so the first batch's nonces are unordered) and the suite returning after serving only the first GetPooledTransactionsMsg request (transaction_suite.go:474), leaving at least 1744 txs never delivered. The ~255 count matches the 256-hash batch, not the 64 queue cap. The 2000-tx calibration is now allowed to change and the fix is tracked in cmd/devp2p: fixture-mode LargeTxRequest never delivers all 2000 txs (batched retrieval + one-shot response) #3067, so it is out of scope for this PR. This PR deliberately does not claim that the 2000-tx case turns green.

Merge Danger

Door: two-way. Test-harness-only change, trivially revertable.

Blast Radius: suite-only. No node, consensus, txpool, or wire-format behaviour changes; only the prices of transactions generated by devp2p rlpx eth-test change. CI is unaffected except for the two new local tests. The one observable consequence is that suite-generated txs now cost gas at the chain floor rather than 1 wei, which is the intended behaviour.

Close #3062
Ref: #2968

Proposed changes

The ethtest fixture batch was priced at 1 wei while the node enforces a chain-derived minimum gas price (2500x on the hydrated gas2500x fixture). Every generated transaction was silently dropped, so the pooled-transaction conformance case could not pass. This derives both the fixture and live-devnet batches from the same params.GetMinGasPrice schedule the node uses, asserts the built batch clears the floor, and adds local regression tests pinning the 1x/2500x tiers and the pending-block rule.

Types of changes

What types of changes does your code introduce to XDC network?
Put an ✅ in the boxes that apply

  • build: Changes that affect the build system or external dependencies
  • ci: Changes to CI configuration files and scripts
  • chore: Changes that don't change source code or tests
  • docs: Documentation only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that neither fixes a bug nor adds a feature
  • revert: Revert something
  • style: Changes that do not affect the meaning of the code
  • test: Adding missing tests or correcting existing tests

Impacted Components

Which parts of the codebase does this PR touch?
Put an ✅ in the boxes that apply

  • Consensus
  • Account
  • Network
  • Geth
  • Smart Contract
  • External components
  • Not sure (Please specify below)

Only cmd/devp2p/internal/ethtest, the RLPx/devp2p conformance test harness. No node or protocol code is touched.

Checklist

Put an ✅ in the boxes once you have confirmed below actions (or provide reasons on not doing so) that

  • This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage
  • Tested on a private network from the genesis block and monitored the chain operating correctly for multiple epochs.
  • Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet.
  • Tested the backwards compatibility.
  • Tested with XDC nodes running this version co-exist with those running the previous version.
  • Relevant documentation has been updated as part of this PR
  • N/A

Reasons for unchecked items:

  • Private-network multi-epoch soak: not applicable. This changes only the devp2p test harness, not chain operation. A four-node fixture devnet was brought up from genesis (frozen suite target plus three validators for QC quorum) to run the live A/B comparison; it was a conformance-suite run, not a chain-stability soak.
  • End-to-end plan: build devp2p from this branch, start the fixture devnet (Local_DPoS_Setup/run-ethtest-fixture.sh), then run devp2p rlpx eth-test --node <frozen.enode> --chain cmd/devp2p/internal/ethtest/testdata/xdc --tap --run '^LargeTxRequest_Xdc166$'; set MATRIX_SENDER_KEY and MATRIX_SENDER_NONCE to exercise the 48-tx live-devnet branch. The Evidence section records the baseline-vs-fixed node-log comparison.
  • Backwards compatibility: verified at the harness level. No persisted data, config, API, or wire format changes; the only behaviour change is the gas price of suite-generated transactions.
  • Mixed-version node coexistence: not applicable, no node binary or protocol change.
  • Documentation: not applicable. The obsolete hardcoded-multiplier comment was removed with the code it described; no user-facing docs change.
  • N/A is not ticked because the applicable items are individually addressed above.

Summary by CodeRabbit

  • Bug Fixes
    • Transaction test scenarios now use the chain’s minimum gas price, avoiding transactions that are priced below the configured floor.
  • Tests
    • Added coverage for gas-price floors across fork boundaries and for generated transaction counts, nonces, and hash entries.

…inOrg#3062

The fixture batch was priced at 1 wei while the hydrated chain enforces a
gas2500x floor of 2500 x DefaultMinGasPrice. All 2000 transactions were
silently dropped by the txpool, so LargeTxRequest_Xdc166 failed with every
announced hash missing even though the node behaved correctly.

Derive the price from the chain's own fork schedule at the pending block
height, the same source the node uses in txpool validation, and assert the
built batch clears that floor before it is announced. The live-devnet branch
uses the same derivation instead of a hardcoded 2500 multiplier. Local tests
pin the 1x and 2500x tiers and the pending-height boundary so a hardcoded
price cannot silently return.

Close XinFinOrg#3062
Ref: XinFinOrg#2968
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Matrix-mode and fixture-generated transactions now use the chain’s minimum gas price for the next block. The generator rejects signed transactions below that floor. Tests cover pending-block floor selection and generated transaction batches.

Changes: Transaction gas-price floor

Layer / File(s) Summary
Calculate and apply the chain floor
cmd/devp2p/internal/ethtest/transaction_suite.go, cmd/devp2p/internal/ethtest/transaction_suite_test.go
A helper calculates the minimum gas price using the next block number. Matrix-mode and generated transactions use that floor, and the generator rejects signed transactions below it. Tests verify floor selection, batch size, gas prices, nonces, and hash-map entries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related issues

  • Issue 2968: The PR changes fixture-generated transaction pricing for the calibrated batch, addressing part of the issue’s remaining fixture-world scope.
Merge Risk: Minimal · up to aca32

This change only adjusts how test transactions are priced in the devp2p ethtest suite. It has no production impact, and no merge-blocking risk was found.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: fixture transactions in the devp2p eth-test suite are now priced at the chain gas floor, and it references the closed issue #3062. It is specific and matches the chang…
Linked Issues check Passed Issue #3062 has three coding acceptance criteria. (1) Derive the price from the chain's fork schedule through params.GetMinGasPrice: met. The diff adds chainGasPriceFloor(), which computes head+1 and …
Out of Scope Changes check Passed Whole-PR diff from merge base 4001230 to head aca3219 covers cmd/devp2p/internal/ethtest/transaction_suite.go (import of params, chainGasPriceFloor helper, matrix-branch pricing, fixture-branch pric…
Description check Passed The description follows the repository template closely. It has a clear summary, the Types of changes section (fix and test checked), Impacted Components (Network checked, with a scope note), and a ch…
Full details: Title check

Explanation

The title names the main change: fixture transactions in the devp2p eth-test suite are now priced at the chain gas floor, and it references the closed issue #3062. It is specific and matches the changeset, which derives transaction prices from params.GetMinGasPrice and adds tests for the floor.

Full details: Linked Issues check

Explanation

Issue #3062 has three coding acceptance criteria. (1) Derive the price from the chain's fork schedule through params.GetMinGasPrice: met. The diff adds chainGasPriceFloor(), which computes head+1 and calls params.GetMinGasPrice(pending, s.chain.config). generateTxs now uses this floor instead of big.NewInt(1). (2) Keep the 2000-transaction calibration: met. The diff does not change the numTxs value or the caller. (3) Assert the built batch clears the chain floor before announcement: met. generateTxs returns an error when signed.GasPriceIntCmp(floor) < 0. The matrix branch in TestLargeTxRequest_Xdc166 also uses the same derived floor instead of the hardcoded DefaultMinGasPrice*2500 multiplier, which the issue names as the reference. Regression tests are reported in transaction_suite_test.go (TestGenerateTxsClearsChainGasFloor, TestChainGasPriceFloorUsesPendingNumber). I read the suite diff but did not open the test file, so test behaviour rests on the PR summary. The PR states that the fixture-mode 2000-tx case still fails on the live devnet with nonce too high. The issue does not list that outcome as a coding acceptance criterion. The devnet A/B logs are reported evidence and were not independently verified.

Full details: Out of Scope Changes check

Explanation

Whole-PR diff from merge base 4001230 to head aca3219 covers cmd/devp2p/internal/ethtest/transaction_suite.go (import of params, chainGasPriceFloor helper, matrix-branch pricing, fixture-branch pricing, floor assertion) and transaction_suite_test.go (regression tests). Every hunk in transaction_suite.go maps to issue #3062. The PR description states that no node, txpool, protocol, config, dependency, or generated-code files change. The ignored_files list is empty. The test file change is supporting test coverage for the issue. No unrelated change was found in the inspected diff.

Full details: Description check

Explanation

The description follows the repository template closely. It has a clear summary, the Types of changes section (fix and test checked), Impacted Components (Network checked, with a scope note), and a checklist with justified reasons for unchecked items. It explains the root cause (fixture batch priced at 1 wei below the chain gas floor), the fix (derive the floor via params.GetMinGasPrice at the pending height, assert the batch clears it), test coverage, and an end-to-end test plan. It also records a known residual: the fixture-mode 2000-tx case still fails with nonce too high, deferred to #3067. Minor gaps: the 'Tested on a private network from the genesis block' box is unchecked with a reason given, and the Documentation item is marked not applicable with a reason. The reasons are provided, so the description is complete.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 4001230 and aca3219.

Files selected for processing: 2

  • cmd/devp2p/internal/ethtest/transaction_suite.go
  • cmd/devp2p/internal/ethtest/transaction_suite_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@gzliudan

Copy link
Copy Markdown
Collaborator Author

No code changes were needed here: the automated review produced no actionable comments and there are no open review threads on this PR. The fixture-mode 2000-tx delivery gap is tracked separately in #3067 and fixed in PR #3068; this PR stays scoped to the chain gas floor. Validation for the delivery work is reported on #3068.

This branch has not been deployed

No deployments
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.

1 participant