From 10abea4561f1a1592b7a14c1aec43950366201d4 Mon Sep 17 00:00:00 2001 From: aBear Date: Wed, 16 Oct 2024 21:40:13 +0200 Subject: [PATCH 1/2] hardened NewU256FromBigInt --- mod/primitives/pkg/math/u256.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mod/primitives/pkg/math/u256.go b/mod/primitives/pkg/math/u256.go index 4f10344c84..e612e5fa2e 100644 --- a/mod/primitives/pkg/math/u256.go +++ b/mod/primitives/pkg/math/u256.go @@ -36,6 +36,12 @@ func NewU256(v uint64) *U256 { // NewU256FromBigInt creates a new U256 from a big.Int. func NewU256FromBigInt(b *big.Int) *U256 { + // Negative integers ought to be rejected by math.NewU256FromBigInt(b) + // since they cannot be expressed in the U256 type. However this does + // not seem to happen holiman/uint256#115 so guarding here. + if b.Sign() < 0 { + panic("Attempting to create U256 from negative big.Int") + } return uint256.MustFromBig(b) } From 797debf8922aba26f0d3b75a83f189126a6e7a05 Mon Sep 17 00:00:00 2001 From: aBear Date: Wed, 16 Oct 2024 22:15:40 +0200 Subject: [PATCH 2/2] NewU256FromBigInt returns error instead of panicking --- mod/cli/pkg/commands/genesis/payload.go | 28 ++++++++---- mod/consensus-types/pkg/types/block.go | 2 +- mod/consensus-types/pkg/types/body.go | 4 +- mod/consensus-types/pkg/types/genesis.go | 37 ++++++++++------ mod/primitives/pkg/math/u256.go | 12 ++++-- mod/primitives/pkg/math/u64_test.go | 54 +++++++++++++++++------- 6 files changed, 91 insertions(+), 46 deletions(-) diff --git a/mod/cli/pkg/commands/genesis/payload.go b/mod/cli/pkg/commands/genesis/payload.go index deccc019ad..45e920020d 100644 --- a/mod/cli/pkg/commands/genesis/payload.go +++ b/mod/cli/pkg/commands/genesis/payload.go @@ -21,6 +21,7 @@ package genesis import ( + "fmt" "unsafe" "github.com/berachain/beacon-kit/mod/cli/pkg/context" @@ -93,11 +94,15 @@ func AddExecutionPayloadCmd(chainSpec common.ChainSpec) *cobra.Command { } // Inject the execution payload. - genesisInfo.ExecutionPayloadHeader = executableDataToExecutionPayloadHeader( - version.ToUint32(genesisInfo.ForkVersion), - payload, - chainSpec.MaxWithdrawalsPerPayload(), - ) + genesisInfo.ExecutionPayloadHeader, err = + executableDataToExecutionPayloadHeader( + version.ToUint32(genesisInfo.ForkVersion), + payload, + chainSpec.MaxWithdrawalsPerPayload(), + ) + if err != nil { + return errors.Wrap(err, "failed to unmarshal beacon state") + } appGenesisState["beacon"], err = json.Marshal(genesisInfo) if err != nil { @@ -124,7 +129,7 @@ func executableDataToExecutionPayloadHeader( data *gethprimitives.ExecutableData, // todo: re-enable when codec supports. _ uint64, -) *types.ExecutionPayloadHeader { +) (*types.ExecutionPayloadHeader, error) { var executionPayloadHeader *types.ExecutionPayloadHeader switch forkVersion { case version.Deneb, version.DenebPlus: @@ -155,6 +160,11 @@ func executableDataToExecutionPayloadHeader( excessBlobGas = *data.ExcessBlobGas } + baseFeePerGas, err := math.NewU256FromBigInt(data.BaseFeePerGas) + if err != nil { + return nil, fmt.Errorf("failed baseFeePerGas conversion: %w", err) + } + executionPayloadHeader = &types.ExecutionPayloadHeader{ ParentHash: common.ExecutionHash(data.ParentHash), FeeRecipient: common.ExecutionAddress(data.FeeRecipient), @@ -167,7 +177,7 @@ func executableDataToExecutionPayloadHeader( GasUsed: math.U64(data.GasUsed), Timestamp: math.U64(data.Timestamp), ExtraData: data.ExtraData, - BaseFeePerGas: math.NewU256FromBigInt(data.BaseFeePerGas), + BaseFeePerGas: baseFeePerGas, BlockHash: common.ExecutionHash(data.BlockHash), // TODO: Decouple from broken bArtio. TransactionsRoot: engineprimitives. @@ -180,8 +190,8 @@ func executableDataToExecutionPayloadHeader( ExcessBlobGas: math.U64(excessBlobGas), } default: - panic("unsupported fork version") + return nil, types.ErrForkVersionNotSupported } - return executionPayloadHeader + return executionPayloadHeader, nil } diff --git a/mod/consensus-types/pkg/types/block.go b/mod/consensus-types/pkg/types/block.go index 6f0b828b97..aced2a55b4 100644 --- a/mod/consensus-types/pkg/types/block.go +++ b/mod/consensus-types/pkg/types/block.go @@ -87,7 +87,7 @@ func (b *BeaconBlock) NewFromSSZ( block = &BeaconBlock{} return block, block.UnmarshalSSZ(bz) case version.DenebPlus: - panic("unsupported fork version") + panic(ErrForkVersionNotSupported) default: return block, ErrForkVersionNotSupported } diff --git a/mod/consensus-types/pkg/types/body.go b/mod/consensus-types/pkg/types/body.go index 456108eba6..299ed9ac8b 100644 --- a/mod/consensus-types/pkg/types/body.go +++ b/mod/consensus-types/pkg/types/body.go @@ -63,7 +63,7 @@ func (b *BeaconBlockBody) Empty(forkVersion uint32) *BeaconBlockBody { }, } default: - panic("unsupported fork version") + panic(ErrForkVersionNotSupported) } } @@ -78,7 +78,7 @@ func BlockBodyKZGOffset( case version.Deneb: return KZGMerkleIndexDeneb * cs.MaxBlobCommitmentsPerBlock() default: - panic("unsupported fork version") + panic(ErrForkVersionNotSupported) } } diff --git a/mod/consensus-types/pkg/types/genesis.go b/mod/consensus-types/pkg/types/genesis.go index 0d3877256a..c4e93ceeaa 100644 --- a/mod/consensus-types/pkg/types/genesis.go +++ b/mod/consensus-types/pkg/types/genesis.go @@ -34,6 +34,11 @@ import ( "github.com/berachain/beacon-kit/mod/primitives/pkg/version" ) +const ( + defaultGasLimit = math.U64(30000000) + defaultBaseFeePerGas = int64(3906250) +) + // Genesis is a struct that contains the genesis information // need to start the beacon chain. // @@ -149,21 +154,25 @@ func DefaultGenesisExecutionPayloadHeaderDeneb() ( if err != nil { return nil, fmt.Errorf("failed generating receipts root: %w", err) } + + baseFeePerGas, err := math.NewU256FromBigInt(big.NewInt(defaultBaseFeePerGas)) + if err != nil { + return nil, fmt.Errorf("failed setting base fee per gas: %w", err) + } + return &ExecutionPayloadHeader{ - ParentHash: common.ExecutionHash{}, - FeeRecipient: common.ExecutionAddress{}, - StateRoot: stateRoot, - ReceiptsRoot: receiptsRoot, - LogsBloom: [256]byte{}, - Random: common.Bytes32{}, - Number: 0, - //nolint:mnd // default value. - GasLimit: math.U64(30000000), - GasUsed: 0, - Timestamp: 0, - ExtraData: make([]byte, constants.ExtraDataLength), - //nolint:mnd // default value. - BaseFeePerGas: math.NewU256FromBigInt(big.NewInt(3906250)), + ParentHash: common.ExecutionHash{}, + FeeRecipient: common.ExecutionAddress{}, + StateRoot: stateRoot, + ReceiptsRoot: receiptsRoot, + LogsBloom: [256]byte{}, + Random: common.Bytes32{}, + Number: 0, + GasLimit: defaultGasLimit, + GasUsed: 0, + Timestamp: 0, + ExtraData: make([]byte, constants.ExtraDataLength), + BaseFeePerGas: baseFeePerGas, BlockHash: common.NewExecutionHashFromHex( "0xcfff92cd918a186029a847b59aca4f83d3941df5946b06bca8de0861fc5d0850", ), diff --git a/mod/primitives/pkg/math/u256.go b/mod/primitives/pkg/math/u256.go index e612e5fa2e..fafd9e0c36 100644 --- a/mod/primitives/pkg/math/u256.go +++ b/mod/primitives/pkg/math/u256.go @@ -21,6 +21,7 @@ package math import ( + "fmt" "math/big" "github.com/holiman/uint256" @@ -35,14 +36,17 @@ func NewU256(v uint64) *U256 { } // NewU256FromBigInt creates a new U256 from a big.Int. -func NewU256FromBigInt(b *big.Int) *U256 { +func NewU256FromBigInt(b *big.Int) (*U256, error) { // Negative integers ought to be rejected by math.NewU256FromBigInt(b) // since they cannot be expressed in the U256 type. However this does - // not seem to happen holiman/uint256#115 so guarding here. + // not seem to happen (see holiman/uint256#115), so guarding here. if b.Sign() < 0 { - panic("Attempting to create U256 from negative big.Int") + return nil, fmt.Errorf( + "cannot convert negative big.Int %s to uint256", + b.String(), + ) } - return uint256.MustFromBig(b) + return uint256.MustFromBig(b), nil } // U256Hex represents a 256-bit unsigned integer that is marshaled to JSON diff --git a/mod/primitives/pkg/math/u64_test.go b/mod/primitives/pkg/math/u64_test.go index c9510377b7..d7b944133d 100644 --- a/mod/primitives/pkg/math/u64_test.go +++ b/mod/primitives/pkg/math/u64_test.go @@ -368,40 +368,62 @@ func TestGwei_ToWei(t *testing.T) { tests := []struct { name string input math.Gwei - expected *math.U256 + expected func(t *testing.T) *math.U256 }{ { - name: "zero gwei", - input: math.Gwei(0), - expected: math.NewU256FromBigInt(big.NewInt(0)), + name: "zero gwei", + input: math.Gwei(0), + expected: func(t *testing.T) *math.U256 { + t.Helper() + res, err := math.NewU256FromBigInt(big.NewInt(0)) + require.NoError(t, err) + return res + }, }, { - name: "one gwei", - input: math.Gwei(1), - expected: math.NewU256FromBigInt(big.NewInt(math.GweiPerWei)), + name: "one gwei", + input: math.Gwei(1), + expected: func(t *testing.T) *math.U256 { + t.Helper() + res, err := math.NewU256FromBigInt(big.NewInt(math.GweiPerWei)) + require.NoError(t, err) + return res + }, }, { name: "arbitrary gwei", input: math.Gwei(123456789), - expected: math.NewU256FromBigInt(new(big.Int).Mul( - big.NewInt(math.GweiPerWei), - big.NewInt(123456789), - )), + expected: func(t *testing.T) *math.U256 { + t.Helper() + n := new(big.Int).Mul( + big.NewInt(math.GweiPerWei), + big.NewInt(123456789), + ) + res, err := math.NewU256FromBigInt(n) + require.NoError(t, err) + return res + }, }, { name: "max uint64 gwei", input: math.Gwei(1<<64 - 1), - expected: math.NewU256FromBigInt(new(big.Int).Mul( - big.NewInt(math.GweiPerWei), - new(big.Int).SetUint64(1<<64-1), - )), + expected: func(t *testing.T) *math.U256 { + t.Helper() + n := new(big.Int).Mul( + big.NewInt(math.GweiPerWei), + new(big.Int).SetUint64(1<<64-1), + ) + res, err := math.NewU256FromBigInt(n) + require.NoError(t, err) + return res + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := tt.input.ToWei() - require.Equal(t, tt.expected, result, "Test case: %s", tt.name) + require.Equal(t, tt.expected(t), result) }) } }