From ac2da12e2f1e0c118b2b3753477d110accf1e8de Mon Sep 17 00:00:00 2001 From: Cal Bera Date: Fri, 15 Aug 2025 10:17:50 -0700 Subject: [PATCH] BRG4-20.3: pass current block number in to NewPoLTx for more clarity --- core/block_validator.go | 3 +- core/block_validator_test.go | 8 +++--- core/types/tx_pol.go | 7 +++-- core/types/tx_pol_test.go | 53 ++++++++++++++++++++++++++++-------- internal/ethapi/simulate.go | 2 +- miner/worker.go | 2 +- 6 files changed, 54 insertions(+), 21 deletions(-) diff --git a/core/block_validator.go b/core/block_validator.go index 6e18e0e2f6..3c129d5ffd 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -19,7 +19,6 @@ package core import ( "errors" "fmt" - "math/big" "github.com/davecgh/go-spew/spew" "github.com/ethereum/go-ethereum/consensus" @@ -100,7 +99,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { polTx, err := types.NewPoLTx( v.config.ChainID, v.config.Berachain.Prague1.PoLDistributorAddress, - new(big.Int).Sub(block.Number(), big.NewInt(1)), + block.Number(), params.PoLTxGasLimit, block.BaseFee(), block.ProposerPubkey(), diff --git a/core/block_validator_test.go b/core/block_validator_test.go index 99e6f3bca7..ba6de304c9 100644 --- a/core/block_validator_test.go +++ b/core/block_validator_test.go @@ -93,7 +93,7 @@ func TestValidateBody_Prague1_Valid(t *testing.T) { chain, validator := buildTestChain(t, cfg) // Build PoL tx + dummy tx. - polTx, err := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(0), params.PoLTxGasLimit, big.NewInt(1000000000), samplePubkey()) + polTx, err := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(1), params.PoLTxGasLimit, big.NewInt(1000000000), samplePubkey()) if err != nil { t.Fatalf("failed to create PoL tx: %v", err) } @@ -113,7 +113,7 @@ func TestValidateBody_Prague1_InvalidHash(t *testing.T) { // PoL tx with WRONG pubkey (different from header.ParentProposerPubkey). wrongPk := &common.Pubkey{} - polTx, _ := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(0), params.PoLTxGasLimit, big.NewInt(1000000000), wrongPk) + polTx, _ := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(1), params.PoLTxGasLimit, big.NewInt(1000000000), wrongPk) block := makeBlock(chain.CurrentHeader(), types.Transactions{polTx}, 1) if err := validator.ValidateBody(block); err == nil { @@ -126,7 +126,7 @@ func TestValidateBody_Prague1_MisplacedPoL(t *testing.T) { cfg := newPrague1Config(distributor) chain, validator := buildTestChain(t, cfg) - polTx, _ := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(0), params.PoLTxGasLimit, big.NewInt(1000000000), samplePubkey()) + polTx, _ := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(1), params.PoLTxGasLimit, big.NewInt(1000000000), samplePubkey()) dummyTx := types.NewTx(&types.LegacyTx{Nonce: 1}) // PoL tx placed second. block := makeBlock(chain.CurrentHeader(), types.Transactions{dummyTx, polTx}, 1) @@ -147,7 +147,7 @@ func TestValidateBody_PrePrague1_PoLProhibited(t *testing.T) { cfg.Berachain.Prague1.PoLDistributorAddress = distributor chain, validator := buildTestChain(t, &cfg) - polTx, _ := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(0), params.PoLTxGasLimit, big.NewInt(1000000000), samplePubkey()) + polTx, _ := types.NewPoLTx(cfg.ChainID, distributor, big.NewInt(1), params.PoLTxGasLimit, big.NewInt(1000000000), samplePubkey()) block := makeBlock(chain.CurrentHeader(), types.Transactions{polTx}, 1) // timestamp 1 < future if err := validator.ValidateBody(block); err == nil { diff --git a/core/types/tx_pol.go b/core/types/tx_pol.go index 839f5d40a5..e15237613a 100644 --- a/core/types/tx_pol.go +++ b/core/types/tx_pol.go @@ -45,7 +45,7 @@ type PoLTx struct { func NewPoLTx( chainID *big.Int, distributorAddress common.Address, - distributionBlockNumber *big.Int, + currentBlockNumber *big.Int, gasLimit uint64, baseFee *big.Int, pubkey *common.Pubkey, @@ -54,11 +54,14 @@ func NewPoLTx( if err != nil { return nil, err } + if currentBlockNumber.Sign() <= 0 { + return nil, errors.New("PoL tx must only be created for a block number greater than 0") + } return NewTx(&PoLTx{ ChainID: chainID, From: params.SystemAddress, To: distributorAddress, - Nonce: distributionBlockNumber.Uint64(), + Nonce: currentBlockNumber.Uint64() - 1, // PoL txs are distributing for the previous block GasLimit: gasLimit, GasPrice: baseFee, Data: data, diff --git a/core/types/tx_pol_test.go b/core/types/tx_pol_test.go index fe70ad3966..e5d9de7aa8 100644 --- a/core/types/tx_pol_test.go +++ b/core/types/tx_pol_test.go @@ -86,28 +86,59 @@ func TestNewPoLTx_NilPubkey(t *testing.T) { } } -// TestNewPoLTx_NegativeBlockNumber ensures negative block numbers are handled -// without panicking (uint64 wrap-around is expected). -func TestNewPoLTx_NegativeBlockNumber(t *testing.T) { +// TestNewPoLTx_InvalidBlockNumber ensures that block numbers <= 0 are rejected. +func TestNewPoLTx_InvalidBlockNumber(t *testing.T) { chainID := big.NewInt(1) distributor := common.Address{} - negBlock := big.NewInt(-1) baseFee := big.NewInt(1000000000) + expectedErr := "PoL tx must only be created for a block number greater than 0" - tx, err := NewPoLTx(chainID, distributor, negBlock, params.PoLTxGasLimit, baseFee, samplePubkey()) - if err != nil { - t.Fatalf("unexpected error: %v", err) + testCases := []struct { + name string + blockNumber *big.Int + }{ + {"negative block number", big.NewInt(-1)}, + {"zero block number", big.NewInt(0)}, } - if got, want := tx.Nonce(), negBlock.Uint64(); got != want { - t.Fatalf("nonce mismatch: have %d, want %d", got, want) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tx, err := NewPoLTx(chainID, distributor, tc.blockNumber, params.PoLTxGasLimit, baseFee, samplePubkey()) + if err == nil { + t.Fatalf("expected error for block number %v, but got nil", tc.blockNumber) + } + if tx != nil { + t.Fatalf("expected nil transaction when error occurs, but got %v", tx) + } + if err.Error() != expectedErr { + t.Fatalf("error message mismatch: have %q, want %q", err.Error(), expectedErr) + } + }) } + + // Test that positive block numbers work correctly + t.Run("positive block number", func(t *testing.T) { + positiveBlock := big.NewInt(123) + tx, err := NewPoLTx(chainID, distributor, positiveBlock, params.PoLTxGasLimit, baseFee, samplePubkey()) + if err != nil { + t.Fatalf("unexpected error for positive block number: %v", err) + } + if tx == nil { + t.Fatalf("expected transaction for positive block number, but got nil") + } + // Nonce should be blockNumber - 1 (as per the implementation) + expectedNonce := positiveBlock.Uint64() - 1 + if got := tx.Nonce(); got != expectedNonce { + t.Fatalf("nonce mismatch: have %d, want %d", got, expectedNonce) + } + }) } // TestIsPoLDistribution exercises positive and negative cases for the helper. func TestIsPoLDistribution(t *testing.T) { distributor := common.HexToAddress("0x1000000000000000000000000000000000000001") baseFee := big.NewInt(1000000000) - tx, err := NewPoLTx(big.NewInt(1), distributor, big.NewInt(0), params.PoLTxGasLimit, baseFee, samplePubkey()) + tx, err := NewPoLTx(big.NewInt(1), distributor, big.NewInt(1), params.PoLTxGasLimit, baseFee, samplePubkey()) if err != nil { t.Fatalf("failed to build PoL tx: %v", err) } @@ -143,7 +174,7 @@ func TestIsPoLDistribution(t *testing.T) { // TestPoLTx_RawSignatureValues confirms that PoLTx reports no signature. func TestPoLTx_RawSignatureValues(t *testing.T) { baseFee := big.NewInt(1000000000) - tx, err := NewPoLTx(big.NewInt(1), common.Address{}, big.NewInt(0), params.PoLTxGasLimit, baseFee, samplePubkey()) + tx, err := NewPoLTx(big.NewInt(1), common.Address{}, big.NewInt(1), params.PoLTxGasLimit, baseFee, samplePubkey()) if err != nil { t.Fatalf("failed to create PoL tx: %v", err) } diff --git a/internal/ethapi/simulate.go b/internal/ethapi/simulate.go index 7f0f6537a7..ca475ec783 100644 --- a/internal/ethapi/simulate.go +++ b/internal/ethapi/simulate.go @@ -207,7 +207,7 @@ func (sim *simulator) execute(ctx context.Context, blocks []simBlock) ([]*simBlo polTx, err := types.NewPoLTx( sim.chainConfig.ChainID, sim.chainConfig.Berachain.Prague1.PoLDistributorAddress, - new(big.Int).Sub(result.Number(), big.NewInt(1)), + result.Number(), params.PoLTxGasLimit, result.BaseFee(), result.ProposerPubkey(), diff --git a/miner/worker.go b/miner/worker.go index 227df41275..1a65d59099 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -502,7 +502,7 @@ func (miner *Miner) fillTransactions(interrupt *atomic.Int32, env *environment) tx, err := types.NewPoLTx( miner.chainConfig.ChainID, miner.chainConfig.Berachain.Prague1.PoLDistributorAddress, - new(big.Int).Sub(env.header.Number, big.NewInt(1)), + env.header.Number, params.PoLTxGasLimit, env.header.BaseFee, env.header.ParentProposerPubkey,