Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(primitives): hardened NewU256FromBigInt #2079

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+95 −44
Open

chore(primitives): hardened NewU256FromBigInt #2079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10abea4
hardened NewU256FromBigInt
abi87 Oct 16, 2024
797debf
NewU256FromBigInt returns error instead of panicking
abi87 Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions mod/cli/pkg/commands/genesis/payload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package genesis

import (
"fmt"
"unsafe"

"github.com/berachain/beacon-kit/mod/cli/pkg/context"
Expand Down Expand Up @@ -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")
}
abi87 marked this conversation as resolved.
Show resolved Hide resolved

appGenesisState["beacon"], err = json.Marshal(genesisInfo)
if err != nil {
Expand All @@ -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:
Expand Down Expand Up @@ -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),
Expand All @@ -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.
Expand All @@ -180,8 +190,8 @@ func executableDataToExecutionPayloadHeader(
ExcessBlobGas: math.U64(excessBlobGas),
}
default:
panic("unsupported fork version")
return nil, types.ErrForkVersionNotSupported
}

return executionPayloadHeader
return executionPayloadHeader, nil
}
2 changes: 1 addition & 1 deletion mod/consensus-types/pkg/types/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (b *BeaconBlock) NewFromSSZ(
block = &BeaconBlock{}
return block, block.UnmarshalSSZ(bz)
case version.DenebPlus:
panic("unsupported fork version")
panic(ErrForkVersionNotSupported)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just found out this error was not consolidated and fixed it. It was used in one of the relevant callers

abi87 marked this conversation as resolved.
Show resolved Hide resolved
default:
return block, ErrForkVersionNotSupported
}
Expand Down
4 changes: 2 additions & 2 deletions mod/consensus-types/pkg/types/body.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (b *BeaconBlockBody) Empty(forkVersion uint32) *BeaconBlockBody {
},
}
default:
panic("unsupported fork version")
panic(ErrForkVersionNotSupported)
}
}

Expand All @@ -78,7 +78,7 @@ func BlockBodyKZGOffset(
case version.Deneb:
return KZGMerkleIndexDeneb * cs.MaxBlobCommitmentsPerBlock()
default:
panic("unsupported fork version")
panic(ErrForkVersionNotSupported)
abi87 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
37 changes: 23 additions & 14 deletions mod/consensus-types/pkg/types/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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)
}
abi87 marked this conversation as resolved.
Show resolved Hide resolved

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,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nit: moved magic value to a const. Likely outside PR's scope but I rather have all magic values together in one place, rather than spread in the codebase where we use them.

GasUsed: 0,
Timestamp: 0,
ExtraData: make([]byte, constants.ExtraDataLength),
BaseFeePerGas: baseFeePerGas,
BlockHash: common.NewExecutionHashFromHex(
"0xcfff92cd918a186029a847b59aca4f83d3941df5946b06bca8de0861fc5d0850",
),
Expand Down
14 changes: 12 additions & 2 deletions mod/primitives/pkg/math/u256.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package math

import (
"fmt"
"math/big"

"github.com/holiman/uint256"
Expand All @@ -35,8 +36,17 @@ func NewU256(v uint64) *U256 {
}

// NewU256FromBigInt creates a new U256 from a big.Int.
func NewU256FromBigInt(b *big.Int) *U256 {
return uint256.MustFromBig(b)
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 (see holiman/uint256#115), so guarding here.
if b.Sign() < 0 {
return nil, fmt.Errorf(
"cannot convert negative big.Int %s to uint256",
b.String(),
)
}
return uint256.MustFromBig(b), nil
}

// U256Hex represents a 256-bit unsigned integer that is marshaled to JSON
Expand Down
54 changes: 38 additions & 16 deletions mod/primitives/pkg/math/u64_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
abi87 marked this conversation as resolved.
Show resolved Hide resolved
})
}
}
Expand Down
Loading