From 7ee9a6e89f59cee21b5852f5f6ffa2bcfc05a25f Mon Sep 17 00:00:00 2001 From: Martin HS Date: Fri, 5 Apr 2024 19:29:44 +0200 Subject: [PATCH 1/7] signer: implement blob txs sendtxargs, enable blobtx-signing (#28976) This change makes it possible to sign blob transactions --- accounts/external/backend.go | 15 ++- core/types/transaction.go | 20 ++++ core/types/tx_blob.go | 6 ++ internal/ethapi/api.go | 15 ++- internal/ethapi/api_test.go | 7 +- internal/ethapi/transaction_args.go | 4 +- signer/core/api.go | 5 +- signer/core/apitypes/types.go | 141 +++++++++++++++++++++++++--- signer/core/apitypes/types_test.go | 104 +++++++++++++++++++- signer/core/cliui.go | 8 +- signer/fourbyte/validation.go | 5 + 11 files changed, 302 insertions(+), 28 deletions(-) diff --git a/accounts/external/backend.go b/accounts/external/backend.go index 6f1581f9b8..0b336448fc 100644 --- a/accounts/external/backend.go +++ b/accounts/external/backend.go @@ -205,7 +205,7 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio to = &t } args := &apitypes.SendTxArgs{ - Data: &data, + Input: &data, Nonce: hexutil.Uint64(tx.Nonce()), Value: hexutil.Big(*tx.Value()), Gas: hexutil.Uint64(tx.Gas()), @@ -215,7 +215,7 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio switch tx.Type() { case types.LegacyTxType, types.AccessListTxType: args.GasPrice = (*hexutil.Big)(tx.GasPrice()) - case types.DynamicFeeTxType: + case types.DynamicFeeTxType, types.BlobTxType: args.MaxFeePerGas = (*hexutil.Big)(tx.GasFeeCap()) args.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap()) default: @@ -235,6 +235,17 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio accessList := tx.AccessList() args.AccessList = &accessList } + if tx.Type() == types.BlobTxType { + args.BlobHashes = tx.BlobHashes() + sidecar := tx.BlobTxSidecar() + if sidecar == nil { + return nil, fmt.Errorf("blobs must be present for signing") + } + args.Blobs = sidecar.Blobs + args.Commitments = sidecar.Commitments + args.Proofs = sidecar.Proofs + } + var res signTransactionResult if err := api.client.Call(&res, "account_signTransaction", args); err != nil { return nil, err diff --git a/core/types/transaction.go b/core/types/transaction.go index d6de9ae73e..6a27ecbfec 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -446,6 +446,26 @@ func (tx *Transaction) WithoutBlobTxSidecar() *Transaction { return cpy } +// WithBlobTxSidecar returns a copy of tx with the blob sidecar added. +func (tx *Transaction) WithBlobTxSidecar(sideCar *BlobTxSidecar) *Transaction { + blobtx, ok := tx.inner.(*BlobTx) + if !ok { + return tx + } + cpy := &Transaction{ + inner: blobtx.withSidecar(sideCar), + time: tx.time, + } + // Note: tx.size cache not carried over because the sidecar is included in size! + if h := tx.hash.Load(); h != nil { + cpy.hash.Store(h) + } + if f := tx.from.Load(); f != nil { + cpy.from.Store(f) + } + return cpy +} + // SetTime sets the decoding time of a transaction. This is used by tests to set // arbitrary times and by persistent transaction pools when loading old txs from // disk. diff --git a/core/types/tx_blob.go b/core/types/tx_blob.go index 25a85695ef..ce1f287caa 100644 --- a/core/types/tx_blob.go +++ b/core/types/tx_blob.go @@ -191,6 +191,12 @@ func (tx *BlobTx) withoutSidecar() *BlobTx { return &cpy } +func (tx *BlobTx) withSidecar(sideCar *BlobTxSidecar) *BlobTx { + cpy := *tx + cpy.Sidecar = sideCar + return &cpy +} + func (tx *BlobTx) encode(b *bytes.Buffer) error { if tx.Sidecar == nil { return rlp.Encode(b, tx) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index c5a99588e6..f682f27658 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1865,15 +1865,14 @@ type SignTransactionResult struct { // The node needs to have the private key of the account corresponding with // the given from address and it needs to be unlocked. func (s *TransactionAPI) SignTransaction(ctx context.Context, args TransactionArgs) (*SignTransactionResult, error) { + args.blobSidecarAllowed = true + if args.Gas == nil { return nil, errors.New("gas not specified") } if args.GasPrice == nil && (args.MaxPriorityFeePerGas == nil || args.MaxFeePerGas == nil) { return nil, errors.New("missing gasPrice or maxFeePerGas/maxPriorityFeePerGas") } - if args.IsEIP4844() { - return nil, errBlobTxNotSupported - } if args.Nonce == nil { return nil, errors.New("nonce not specified") } @@ -1889,6 +1888,16 @@ func (s *TransactionAPI) SignTransaction(ctx context.Context, args TransactionAr if err != nil { return nil, err } + // If the transaction-to-sign was a blob transaction, then the signed one + // no longer retains the blobs, only the blob hashes. In this step, we need + // to put back the blob(s). + if args.IsEIP4844() { + signed = signed.WithBlobTxSidecar(&types.BlobTxSidecar{ + Blobs: args.Blobs, + Commitments: args.Commitments, + Proofs: args.Proofs, + }) + } data, err := signed.MarshalBinary() if err != nil { return nil, err diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index b9ccf2bf7d..6aad4097fe 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -1037,11 +1037,8 @@ func TestSignBlobTransaction(t *testing.T) { } _, err = api.SignTransaction(context.Background(), argsFromTransaction(res.Tx, b.acc.Address)) - if err == nil { - t.Fatalf("should fail on blob transaction") - } - if !errors.Is(err, errBlobTxNotSupported) { - t.Errorf("error mismatch. Have: %v, want: %v", err, errBlobTxNotSupported) + if err != nil { + t.Fatalf("should not fail on blob transaction") } } diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index bef6082ead..f199f9d912 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -97,7 +97,7 @@ func (args *TransactionArgs) data() []byte { // setDefaults fills in default values for unspecified tx fields. func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, skipGasEstimation bool) error { - if err := args.setBlobTxSidecar(ctx, b); err != nil { + if err := args.setBlobTxSidecar(ctx); err != nil { return err } if err := args.setFeeDefaults(ctx, b); err != nil { @@ -290,7 +290,7 @@ func (args *TransactionArgs) setLondonFeeDefaults(ctx context.Context, head *typ } // setBlobTxSidecar adds the blob tx -func (args *TransactionArgs) setBlobTxSidecar(ctx context.Context, b Backend) error { +func (args *TransactionArgs) setBlobTxSidecar(ctx context.Context) error { // No blobs, we're done. if args.Blobs == nil { return nil diff --git a/signer/core/api.go b/signer/core/api.go index a32f24cb18..23ddcd0a20 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -590,7 +590,10 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args apitypes.SendTxA return nil, err } // Convert fields into a real transaction - var unsignedTx = result.Transaction.ToTransaction() + unsignedTx, err := result.Transaction.ToTransaction() + if err != nil { + return nil, err + } // Get the password for the transaction pw, err := api.lookupOrQueryPassword(acc.Address, "Account password", fmt.Sprintf("Please enter the password for account %s", acc.Address.String())) diff --git a/signer/core/apitypes/types.go b/signer/core/apitypes/types.go index 0d66887d58..2dbe2fc4a6 100644 --- a/signer/core/apitypes/types.go +++ b/signer/core/apitypes/types.go @@ -18,6 +18,7 @@ package apitypes import ( "bytes" + "crypto/sha256" "encoding/json" "errors" "fmt" @@ -34,6 +35,8 @@ import ( "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/kzg4844" + "github.com/holiman/uint256" ) var typedDataReferenceTypeRegexp = regexp.MustCompile(`^[A-Za-z](\w*)(\[\])?$`) @@ -92,12 +95,21 @@ type SendTxArgs struct { // We accept "data" and "input" for backwards-compatibility reasons. // "input" is the newer name and should be preferred by clients. // Issue detail: https://github.com/ethereum/go-ethereum/issues/15628 - Data *hexutil.Bytes `json:"data"` + Data *hexutil.Bytes `json:"data,omitempty"` Input *hexutil.Bytes `json:"input,omitempty"` // For non-legacy transactions AccessList *types.AccessList `json:"accessList,omitempty"` ChainID *hexutil.Big `json:"chainId,omitempty"` + + // For BlobTxType + BlobFeeCap *hexutil.Big `json:"maxFeePerBlobGas,omitempty"` + BlobHashes []common.Hash `json:"blobVersionedHashes,omitempty"` + + // For BlobTxType transactions with blob sidecar + Blobs []kzg4844.Blob `json:"blobs,omitempty"` + Commitments []kzg4844.Commitment `json:"commitments,omitempty"` + Proofs []kzg4844.Proof `json:"proofs,omitempty"` } func (args SendTxArgs) String() string { @@ -108,24 +120,56 @@ func (args SendTxArgs) String() string { return err.Error() } +// data retrieves the transaction calldata. Input field is preferred. +func (args *SendTxArgs) data() []byte { + if args.Input != nil { + return *args.Input + } + if args.Data != nil { + return *args.Data + } + return nil +} + // ToTransaction converts the arguments to a transaction. -func (args *SendTxArgs) ToTransaction() *types.Transaction { +func (args *SendTxArgs) ToTransaction() (*types.Transaction, error) { // Add the To-field, if specified var to *common.Address if args.To != nil { dstAddr := args.To.Address() to = &dstAddr } - - var input []byte - if args.Input != nil { - input = *args.Input - } else if args.Data != nil { - input = *args.Data + if err := args.validateTxSidecar(); err != nil { + return nil, err } - var data types.TxData switch { + case args.BlobHashes != nil: + al := types.AccessList{} + if args.AccessList != nil { + al = *args.AccessList + } + data = &types.BlobTx{ + To: *to, + ChainID: uint256.MustFromBig((*big.Int)(args.ChainID)), + Nonce: uint64(args.Nonce), + Gas: uint64(args.Gas), + GasFeeCap: uint256.MustFromBig((*big.Int)(args.MaxFeePerGas)), + GasTipCap: uint256.MustFromBig((*big.Int)(args.MaxPriorityFeePerGas)), + Value: uint256.MustFromBig((*big.Int)(&args.Value)), + Data: args.data(), + AccessList: al, + BlobHashes: args.BlobHashes, + BlobFeeCap: uint256.MustFromBig((*big.Int)(args.BlobFeeCap)), + } + if args.Blobs != nil { + data.(*types.BlobTx).Sidecar = &types.BlobTxSidecar{ + Blobs: args.Blobs, + Commitments: args.Commitments, + Proofs: args.Proofs, + } + } + case args.MaxFeePerGas != nil: al := types.AccessList{} if args.AccessList != nil { @@ -139,7 +183,7 @@ func (args *SendTxArgs) ToTransaction() *types.Transaction { GasFeeCap: (*big.Int)(args.MaxFeePerGas), GasTipCap: (*big.Int)(args.MaxPriorityFeePerGas), Value: (*big.Int)(&args.Value), - Data: input, + Data: args.data(), AccessList: al, } case args.AccessList != nil: @@ -150,7 +194,7 @@ func (args *SendTxArgs) ToTransaction() *types.Transaction { Gas: uint64(args.Gas), GasPrice: (*big.Int)(args.GasPrice), Value: (*big.Int)(&args.Value), - Data: input, + Data: args.data(), AccessList: *args.AccessList, } default: @@ -160,10 +204,81 @@ func (args *SendTxArgs) ToTransaction() *types.Transaction { Gas: uint64(args.Gas), GasPrice: (*big.Int)(args.GasPrice), Value: (*big.Int)(&args.Value), - Data: input, + Data: args.data(), } } - return types.NewTx(data) + + return types.NewTx(data), nil +} + +// validateTxSidecar validates blob data, if present +func (args *SendTxArgs) validateTxSidecar() error { + // No blobs, we're done. + if args.Blobs == nil { + return nil + } + + n := len(args.Blobs) + // Assume user provides either only blobs (w/o hashes), or + // blobs together with commitments and proofs. + if args.Commitments == nil && args.Proofs != nil { + return errors.New(`blob proofs provided while commitments were not`) + } else if args.Commitments != nil && args.Proofs == nil { + return errors.New(`blob commitments provided while proofs were not`) + } + + // len(blobs) == len(commitments) == len(proofs) == len(hashes) + if args.Commitments != nil && len(args.Commitments) != n { + return fmt.Errorf("number of blobs and commitments mismatch (have=%d, want=%d)", len(args.Commitments), n) + } + if args.Proofs != nil && len(args.Proofs) != n { + return fmt.Errorf("number of blobs and proofs mismatch (have=%d, want=%d)", len(args.Proofs), n) + } + if args.BlobHashes != nil && len(args.BlobHashes) != n { + return fmt.Errorf("number of blobs and hashes mismatch (have=%d, want=%d)", len(args.BlobHashes), n) + } + + if args.Commitments == nil { + // Generate commitment and proof. + commitments := make([]kzg4844.Commitment, n) + proofs := make([]kzg4844.Proof, n) + for i, b := range args.Blobs { + c, err := kzg4844.BlobToCommitment(b) + if err != nil { + return fmt.Errorf("blobs[%d]: error computing commitment: %v", i, err) + } + commitments[i] = c + p, err := kzg4844.ComputeBlobProof(b, c) + if err != nil { + return fmt.Errorf("blobs[%d]: error computing proof: %v", i, err) + } + proofs[i] = p + } + args.Commitments = commitments + args.Proofs = proofs + } else { + for i, b := range args.Blobs { + if err := kzg4844.VerifyBlobProof(b, args.Commitments[i], args.Proofs[i]); err != nil { + return fmt.Errorf("failed to verify blob proof: %v", err) + } + } + } + + hashes := make([]common.Hash, n) + hasher := sha256.New() + for i, c := range args.Commitments { + hashes[i] = kzg4844.CalcBlobHashV1(hasher, &c) + } + if args.BlobHashes != nil { + for i, h := range hashes { + if h != args.BlobHashes[i] { + return fmt.Errorf("blob hash verification failed (have=%s, want=%s)", args.BlobHashes[i], h) + } + } + } else { + args.BlobHashes = hashes + } + return nil } type SigFormat struct { diff --git a/signer/core/apitypes/types_test.go b/signer/core/apitypes/types_test.go index b5aa3d1e93..324ff8a840 100644 --- a/signer/core/apitypes/types_test.go +++ b/signer/core/apitypes/types_test.go @@ -16,7 +16,16 @@ package apitypes -import "testing" +import ( + "crypto/sha256" + "encoding/json" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/crypto/kzg4844" + "github.com/holiman/uint256" +) func TestIsPrimitive(t *testing.T) { t.Parallel() @@ -39,3 +48,96 @@ func TestIsPrimitive(t *testing.T) { } } } + +func TestTxArgs(t *testing.T) { + for i, tc := range []struct { + data []byte + want common.Hash + wantType uint8 + }{ + { + data: []byte(`{"from":"0x1b442286e32ddcaa6e2570ce9ed85f4b4fc87425","accessList":[],"blobVersionedHashes":["0x010657f37554c781402a22917dee2f75def7ab966d7b770905398eba3c444014"],"chainId":"0x7","gas":"0x124f8","gasPrice":"0x693d4ca8","input":"0x","maxFeePerBlobGas":"0x3b9aca00","maxFeePerGas":"0x6fc23ac00","maxPriorityFeePerGas":"0x3b9aca00","nonce":"0x0","r":"0x2a922afc784d07e98012da29f2f37cae1f73eda78aa8805d3df6ee5dbb41ec1","s":"0x4f1f75ae6bcdf4970b4f305da1a15d8c5ddb21f555444beab77c9af2baab14","to":"0x1b442286e32ddcaa6e2570ce9ed85f4b4fc87425","type":"0x1","v":"0x0","value":"0x0","yParity":"0x0"}`), + want: common.HexToHash("0x7d53234acc11ac5b5948632c901a944694e228795782f511887d36fd76ff15c4"), + wantType: types.BlobTxType, + }, + { + // on input, we don't read the type, but infer the type from the arguments present + data: []byte(`{"from":"0x1b442286e32ddcaa6e2570ce9ed85f4b4fc87425","accessList":[],"chainId":"0x7","gas":"0x124f8","gasPrice":"0x693d4ca8","input":"0x","maxFeePerBlobGas":"0x3b9aca00","maxFeePerGas":"0x6fc23ac00","maxPriorityFeePerGas":"0x3b9aca00","nonce":"0x0","r":"0x2a922afc784d07e98012da29f2f37cae1f73eda78aa8805d3df6ee5dbb41ec1","s":"0x4f1f75ae6bcdf4970b4f305da1a15d8c5ddb21f555444beab77c9af2baab14","to":"0x1b442286e32ddcaa6e2570ce9ed85f4b4fc87425","type":"0x12","v":"0x0","value":"0x0","yParity":"0x0"}`), + want: common.HexToHash("0x7919e2b0b9b543cb87a137b6ff66491ec7ae937cb88d3c29db4d9b28073dce53"), + wantType: types.DynamicFeeTxType, + }, + } { + var txArgs SendTxArgs + if err := json.Unmarshal(tc.data, &txArgs); err != nil { + t.Fatal(err) + } + tx, err := txArgs.ToTransaction() + if err != nil { + t.Fatal(err) + } + if have := tx.Type(); have != tc.wantType { + t.Errorf("test %d, have type %d, want type %d", i, have, tc.wantType) + } + if have := tx.Hash(); have != tc.want { + t.Errorf("test %d: have %v, want %v", i, have, tc.want) + } + d2, err := json.Marshal(txArgs) + if err != nil { + t.Fatal(err) + } + var txArgs2 SendTxArgs + if err := json.Unmarshal(d2, &txArgs2); err != nil { + t.Fatal(err) + } + tx1, _ := txArgs.ToTransaction() + tx2, _ := txArgs2.ToTransaction() + if have, want := tx1.Hash(), tx2.Hash(); have != want { + t.Errorf("test %d: have %v, want %v", i, have, want) + } + } + /* + End to end testing: + + $ go run ./cmd/clef --advanced --suppress-bootwarn + + $ go run ./cmd/geth --nodiscover --maxpeers 0 --signer /home/user/.clef/clef.ipc console + + > tx={"from":"0x1b442286e32ddcaa6e2570ce9ed85f4b4fc87425","to":"0x1b442286e32ddcaa6e2570ce9ed85f4b4fc87425","gas":"0x124f8","maxFeePerGas":"0x6fc23ac00","maxPriorityFeePerGas":"0x3b9aca00","value":"0x0","nonce":"0x0","input":"0x","accessList":[],"maxFeePerBlobGas":"0x3b9aca00","blobVersionedHashes":["0x010657f37554c781402a22917dee2f75def7ab966d7b770905398eba3c444014"]} + > eth.signTransaction(tx) + */ +} + +func TestBlobTxs(t *testing.T) { + blob := kzg4844.Blob{0x1} + commitment, err := kzg4844.BlobToCommitment(blob) + if err != nil { + t.Fatal(err) + } + proof, err := kzg4844.ComputeBlobProof(blob, commitment) + if err != nil { + t.Fatal(err) + } + + hash := kzg4844.CalcBlobHashV1(sha256.New(), &commitment) + b := &types.BlobTx{ + ChainID: uint256.NewInt(6), + Nonce: 8, + GasTipCap: uint256.NewInt(500), + GasFeeCap: uint256.NewInt(600), + Gas: 21000, + BlobFeeCap: uint256.NewInt(700), + BlobHashes: []common.Hash{hash}, + Value: uint256.NewInt(100), + Sidecar: &types.BlobTxSidecar{ + Blobs: []kzg4844.Blob{blob}, + Commitments: []kzg4844.Commitment{commitment}, + Proofs: []kzg4844.Proof{proof}, + }, + } + tx := types.NewTx(b) + data, err := json.Marshal(tx) + if err != nil { + t.Fatal(err) + } + t.Logf("tx %v", string(data)) +} diff --git a/signer/core/cliui.go b/signer/core/cliui.go index b1bd3206ed..e04077865d 100644 --- a/signer/core/cliui.go +++ b/signer/core/cliui.go @@ -128,7 +128,7 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro fmt.Printf("chainid: %v\n", chainId) } if list := request.Transaction.AccessList; list != nil { - fmt.Printf("Accesslist\n") + fmt.Printf("Accesslist:\n") for i, el := range *list { fmt.Printf(" %d. %v\n", i, el.Address) for j, slot := range el.StorageKeys { @@ -136,6 +136,12 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro } } } + if len(request.Transaction.BlobHashes) > 0 { + fmt.Printf("Blob hashes:\n") + for _, bh := range request.Transaction.BlobHashes { + fmt.Printf(" %v\n", bh) + } + } if request.Transaction.Data != nil { d := *request.Transaction.Data if len(d) > 0 { diff --git a/signer/fourbyte/validation.go b/signer/fourbyte/validation.go index 58111e8e00..0451bda91d 100644 --- a/signer/fourbyte/validation.go +++ b/signer/fourbyte/validation.go @@ -36,6 +36,11 @@ func (db *Database) ValidateTransaction(selector *string, tx *apitypes.SendTxArg if tx.Data != nil && tx.Input != nil && !bytes.Equal(*tx.Data, *tx.Input) { return nil, errors.New(`ambiguous request: both "data" and "input" are set and are not identical`) } + // ToTransaction validates, among other things, that blob hashes match with blobs, and also + // populates the hashes if they were previously unset. + if _, err := tx.ToTransaction(); err != nil { + return nil, err + } // Place data on 'data', and nil 'input' var data []byte if tx.Input != nil { From 4458905f261d5d9ba5fda3d664f9bb80346ab404 Mon Sep 17 00:00:00 2001 From: Martin HS Date: Fri, 5 Apr 2024 21:01:39 +0200 Subject: [PATCH 2/7] signer/core/apitypes: fix apitypes breakage due to bitrotted PR (#29470) --- signer/core/apitypes/types.go | 6 +++--- signer/core/apitypes/types_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/signer/core/apitypes/types.go b/signer/core/apitypes/types.go index 2dbe2fc4a6..eba9d7768f 100644 --- a/signer/core/apitypes/types.go +++ b/signer/core/apitypes/types.go @@ -243,12 +243,12 @@ func (args *SendTxArgs) validateTxSidecar() error { commitments := make([]kzg4844.Commitment, n) proofs := make([]kzg4844.Proof, n) for i, b := range args.Blobs { - c, err := kzg4844.BlobToCommitment(b) + c, err := kzg4844.BlobToCommitment(&b) if err != nil { return fmt.Errorf("blobs[%d]: error computing commitment: %v", i, err) } commitments[i] = c - p, err := kzg4844.ComputeBlobProof(b, c) + p, err := kzg4844.ComputeBlobProof(&b, c) if err != nil { return fmt.Errorf("blobs[%d]: error computing proof: %v", i, err) } @@ -258,7 +258,7 @@ func (args *SendTxArgs) validateTxSidecar() error { args.Proofs = proofs } else { for i, b := range args.Blobs { - if err := kzg4844.VerifyBlobProof(b, args.Commitments[i], args.Proofs[i]); err != nil { + if err := kzg4844.VerifyBlobProof(&b, args.Commitments[i], args.Proofs[i]); err != nil { return fmt.Errorf("failed to verify blob proof: %v", err) } } diff --git a/signer/core/apitypes/types_test.go b/signer/core/apitypes/types_test.go index 324ff8a840..7ea32f298c 100644 --- a/signer/core/apitypes/types_test.go +++ b/signer/core/apitypes/types_test.go @@ -109,11 +109,11 @@ func TestTxArgs(t *testing.T) { func TestBlobTxs(t *testing.T) { blob := kzg4844.Blob{0x1} - commitment, err := kzg4844.BlobToCommitment(blob) + commitment, err := kzg4844.BlobToCommitment(&blob) if err != nil { t.Fatal(err) } - proof, err := kzg4844.ComputeBlobProof(blob, commitment) + proof, err := kzg4844.ComputeBlobProof(&blob, commitment) if err != nil { t.Fatal(err) } From cc348a601ee816d6c0e2c4d7246c810f3b61e798 Mon Sep 17 00:00:00 2001 From: georgehao Date: Sat, 6 Apr 2024 17:09:30 +0800 Subject: [PATCH 3/7] common/prque: fix godoc comments (#29460) Co-authored-by: Felix Lange --- common/prque/prque.go | 14 +++++++------- common/prque/sstack.go | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/common/prque/prque.go b/common/prque/prque.go index ec8351020a..cb0d9f3580 100755 --- a/common/prque/prque.go +++ b/common/prque/prque.go @@ -22,7 +22,7 @@ import ( "container/heap" ) -// Priority queue data structure. +// Prque is a priority queue data structure. type Prque[P cmp.Ordered, V any] struct { cont *sstack[P, V] } @@ -32,7 +32,7 @@ func New[P cmp.Ordered, V any](setIndex SetIndexCallback[V]) *Prque[P, V] { return &Prque[P, V]{newSstack[P, V](setIndex)} } -// Pushes a value with a given priority into the queue, expanding if necessary. +// Push a value with a given priority into the queue, expanding if necessary. func (p *Prque[P, V]) Push(data V, priority P) { heap.Push(p.cont, &item[P, V]{data, priority}) } @@ -43,14 +43,14 @@ func (p *Prque[P, V]) Peek() (V, P) { return item.value, item.priority } -// Pops the value with the greatest priority off the stack and returns it. +// Pop the value with the greatest priority off the stack and returns it. // Currently no shrinking is done. func (p *Prque[P, V]) Pop() (V, P) { item := heap.Pop(p.cont).(*item[P, V]) return item.value, item.priority } -// Pops only the item from the queue, dropping the associated priority value. +// PopItem pops only the item from the queue, dropping the associated priority value. func (p *Prque[P, V]) PopItem() V { return heap.Pop(p.cont).(*item[P, V]).value } @@ -60,17 +60,17 @@ func (p *Prque[P, V]) Remove(i int) V { return heap.Remove(p.cont, i).(*item[P, V]).value } -// Checks whether the priority queue is empty. +// Empty checks whether the priority queue is empty. func (p *Prque[P, V]) Empty() bool { return p.cont.Len() == 0 } -// Returns the number of element in the priority queue. +// Size returns the number of element in the priority queue. func (p *Prque[P, V]) Size() int { return p.cont.Len() } -// Clears the contents of the priority queue. +// Reset clears the contents of the priority queue. func (p *Prque[P, V]) Reset() { *p = *New[P, V](p.cont.setIndex) } diff --git a/common/prque/sstack.go b/common/prque/sstack.go index ee6d7c0c3a..6865a51e23 100755 --- a/common/prque/sstack.go +++ b/common/prque/sstack.go @@ -49,7 +49,7 @@ func newSstack[P cmp.Ordered, V any](setIndex SetIndexCallback[V]) *sstack[P, V] return result } -// Pushes a value onto the stack, expanding it if necessary. Required by +// Push a value onto the stack, expanding it if necessary. Required by // heap.Interface. func (s *sstack[P, V]) Push(data any) { if s.size == s.capacity { @@ -69,7 +69,7 @@ func (s *sstack[P, V]) Push(data any) { s.size++ } -// Pops a value off the stack and returns it. Currently no shrinking is done. +// Pop a value off the stack and returns it. Currently no shrinking is done. // Required by heap.Interface. func (s *sstack[P, V]) Pop() (res any) { s.size-- @@ -85,18 +85,18 @@ func (s *sstack[P, V]) Pop() (res any) { return } -// Returns the length of the stack. Required by sort.Interface. +// Len returns the length of the stack. Required by sort.Interface. func (s *sstack[P, V]) Len() int { return s.size } -// Compares the priority of two elements of the stack (higher is first). +// Less compares the priority of two elements of the stack (higher is first). // Required by sort.Interface. func (s *sstack[P, V]) Less(i, j int) bool { return s.blocks[i/blockSize][i%blockSize].priority > s.blocks[j/blockSize][j%blockSize].priority } -// Swaps two elements in the stack. Required by sort.Interface. +// Swap two elements in the stack. Required by sort.Interface. func (s *sstack[P, V]) Swap(i, j int) { ib, io, jb, jo := i/blockSize, i%blockSize, j/blockSize, j%blockSize a, b := s.blocks[jb][jo], s.blocks[ib][io] @@ -107,7 +107,7 @@ func (s *sstack[P, V]) Swap(i, j int) { s.blocks[ib][io], s.blocks[jb][jo] = a, b } -// Resets the stack, effectively clearing its contents. +// Reset the stack, effectively clearing its contents. func (s *sstack[P, V]) Reset() { *s = *newSstack[P, V](s.setIndex) } From 74995bf8a169bb9d07333e56623ea039b8664710 Mon Sep 17 00:00:00 2001 From: Aaron Chen Date: Sat, 6 Apr 2024 18:05:06 +0800 Subject: [PATCH 4/7] all: use slices.Contains (#29459) Co-authored-by: Felix Lange --- cmd/devp2p/internal/v5test/discv5tests.go | 3 ++- cmd/devp2p/internal/v5test/framework.go | 9 --------- eth/filters/filter.go | 15 +++------------ node/node.go | 13 ++----------- node/node_test.go | 3 ++- 5 files changed, 9 insertions(+), 34 deletions(-) diff --git a/cmd/devp2p/internal/v5test/discv5tests.go b/cmd/devp2p/internal/v5test/discv5tests.go index 56624a0ca8..7dbd3c3be5 100644 --- a/cmd/devp2p/internal/v5test/discv5tests.go +++ b/cmd/devp2p/internal/v5test/discv5tests.go @@ -19,6 +19,7 @@ package v5test import ( "bytes" "net" + "slices" "sync" "time" @@ -266,7 +267,7 @@ func (s *Suite) TestFindnodeResults(t *utesting.T) { n := bn.conn.localNode.Node() expect[n.ID()] = n d := uint(enode.LogDist(n.ID(), s.Dest.ID())) - if !containsUint(dists, d) { + if !slices.Contains(dists, d) { dists = append(dists, d) } } diff --git a/cmd/devp2p/internal/v5test/framework.go b/cmd/devp2p/internal/v5test/framework.go index 10856a50bc..92a5048150 100644 --- a/cmd/devp2p/internal/v5test/framework.go +++ b/cmd/devp2p/internal/v5test/framework.go @@ -252,12 +252,3 @@ func checkRecords(records []*enr.Record) ([]*enode.Node, error) { } return nodes, nil } - -func containsUint(ints []uint, x uint) bool { - for i := range ints { - if ints[i] == x { - return true - } - } - return false -} diff --git a/eth/filters/filter.go b/eth/filters/filter.go index f2b92d5a99..2f59026b73 100644 --- a/eth/filters/filter.go +++ b/eth/filters/filter.go @@ -20,6 +20,7 @@ import ( "context" "errors" "math/big" + "slices" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/bloombits" @@ -347,16 +348,6 @@ func (f *Filter) pendingLogs() []*types.Log { return nil } -// includes returns true if the element is present in the list. -func includes[T comparable](things []T, element T) bool { - for _, thing := range things { - if thing == element { - return true - } - } - return false -} - // filterLogs creates a slice of logs matching the given criteria. func filterLogs(logs []*types.Log, fromBlock, toBlock *big.Int, addresses []common.Address, topics [][]common.Hash) []*types.Log { var check = func(log *types.Log) bool { @@ -366,7 +357,7 @@ func filterLogs(logs []*types.Log, fromBlock, toBlock *big.Int, addresses []comm if toBlock != nil && toBlock.Int64() >= 0 && toBlock.Uint64() < log.BlockNumber { return false } - if len(addresses) > 0 && !includes(addresses, log.Address) { + if len(addresses) > 0 && !slices.Contains(addresses, log.Address) { return false } // If the to filtered topics is greater than the amount of topics in logs, skip. @@ -377,7 +368,7 @@ func filterLogs(logs []*types.Log, fromBlock, toBlock *big.Int, addresses []comm if len(sub) == 0 { continue // empty rule set == wildcard } - if !includes(sub, log.Topics[i]) { + if !slices.Contains(sub, log.Topics[i]) { return false } } diff --git a/node/node.go b/node/node.go index c5cb552d27..6cbae68591 100644 --- a/node/node.go +++ b/node/node.go @@ -25,6 +25,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "strings" "sync" @@ -278,16 +279,6 @@ func (n *Node) openEndpoints() error { return err } -// containsLifecycle checks if 'lfs' contains 'l'. -func containsLifecycle(lfs []Lifecycle, l Lifecycle) bool { - for _, obj := range lfs { - if obj == l { - return true - } - } - return false -} - // stopServices terminates running services, RPC and p2p networking. // It is the inverse of Start. func (n *Node) stopServices(running []Lifecycle) error { @@ -571,7 +562,7 @@ func (n *Node) RegisterLifecycle(lifecycle Lifecycle) { if n.state != initializingState { panic("can't register lifecycle on running/stopped node") } - if containsLifecycle(n.lifecycles, lifecycle) { + if slices.Contains(n.lifecycles, lifecycle) { panic(fmt.Sprintf("attempt to register lifecycle %T more than once", lifecycle)) } n.lifecycles = append(n.lifecycles, lifecycle) diff --git a/node/node_test.go b/node/node_test.go index d1d1e5dfe8..82e814cada 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -23,6 +23,7 @@ import ( "net" "net/http" "reflect" + "slices" "strings" "testing" @@ -116,7 +117,7 @@ func TestLifecycleRegistry_Successful(t *testing.T) { noop := NewNoop() stack.RegisterLifecycle(noop) - if !containsLifecycle(stack.lifecycles, noop) { + if !slices.Contains(stack.lifecycles, Lifecycle(noop)) { t.Fatalf("lifecycle was not properly registered on the node, %v", err) } } From ccb76c01d7b1ce4d77d2bb309419cc78f42659ca Mon Sep 17 00:00:00 2001 From: Aaron Chen Date: Sat, 6 Apr 2024 18:16:25 +0800 Subject: [PATCH 5/7] eth/tracers: use slices.Contains (#29461) --- eth/tracers/js/goja.go | 9 ++------- eth/tracers/native/call_flat.go | 8 ++------ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/eth/tracers/js/goja.go b/eth/tracers/js/goja.go index 82666155ec..5290d4f709 100644 --- a/eth/tracers/js/goja.go +++ b/eth/tracers/js/goja.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "math/big" + "slices" "github.com/dop251/goja" "github.com/ethereum/go-ethereum/core/tracing" @@ -529,13 +530,7 @@ func (t *jsTracer) setBuiltinFunctions() { vm.Interrupt(err) return false } - addr := common.BytesToAddress(a) - for _, p := range t.activePrecompiles { - if p == addr { - return true - } - } - return false + return slices.Contains(t.activePrecompiles, common.BytesToAddress(a)) }) vm.Set("slice", func(slice goja.Value, start, end int64) goja.Value { b, err := t.fromBuf(vm, slice, false) diff --git a/eth/tracers/native/call_flat.go b/eth/tracers/native/call_flat.go index 37be64310c..f8d38ddd2d 100644 --- a/eth/tracers/native/call_flat.go +++ b/eth/tracers/native/call_flat.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "math/big" + "slices" "strings" "github.com/ethereum/go-ethereum/common" @@ -228,12 +229,7 @@ func (t *flatCallTracer) Stop(err error) { // isPrecompiled returns whether the addr is a precompile. func (t *flatCallTracer) isPrecompiled(addr common.Address) bool { - for _, p := range t.activePrecompiles { - if p == addr { - return true - } - } - return false + return slices.Contains(t.activePrecompiles, addr) } func flatFromNested(input *callFrame, traceAddress []int, convertErrs bool, ctx *tracers.Context) (output []flatCallFrame, err error) { From 8876868bb831cef307d7e72c6848bd0943ba1e24 Mon Sep 17 00:00:00 2001 From: Roberto Bayardo Date: Sat, 6 Apr 2024 03:17:41 -0700 Subject: [PATCH 6/7] log: default JSON log handler should log all verbosity levels (#29471) Co-authored-by: lightclient --- internal/debug/flags.go | 4 ++-- log/handler.go | 7 +++++++ log/logger_test.go | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/debug/flags.go b/internal/debug/flags.go index 19222c8325..361dc6fcca 100644 --- a/internal/debug/flags.go +++ b/internal/debug/flags.go @@ -231,9 +231,9 @@ func Setup(ctx *cli.Context) error { case ctx.Bool(logjsonFlag.Name): // Retain backwards compatibility with `--log.json` flag if `--log.format` not set defer log.Warn("The flag '--log.json' is deprecated, please use '--log.format=json' instead") - handler = log.JSONHandler(output) + handler = log.JSONHandlerWithLevel(output, log.LevelInfo) case logFmtFlag == "json": - handler = log.JSONHandler(output) + handler = log.JSONHandlerWithLevel(output, log.LevelInfo) case logFmtFlag == "logfmt": handler = log.LogfmtHandler(output) case logFmtFlag == "", logFmtFlag == "terminal": diff --git a/log/handler.go b/log/handler.go index 248e3813fc..290e381509 100644 --- a/log/handler.go +++ b/log/handler.go @@ -115,8 +115,15 @@ func (l *leveler) Level() slog.Level { // JSONHandler returns a handler which prints records in JSON format. func JSONHandler(wr io.Writer) slog.Handler { + return JSONHandlerWithLevel(wr, levelMaxVerbosity) +} + +// JSONHandler returns a handler which prints records in JSON format that are less than or equal to +// the specified verbosity level. +func JSONHandlerWithLevel(wr io.Writer, level slog.Level) slog.Handler { return slog.NewJSONHandler(wr, &slog.HandlerOptions{ ReplaceAttr: builtinReplaceJSON, + Level: &leveler{level}, }) } diff --git a/log/logger_test.go b/log/logger_test.go index d23e16e572..2ea0858547 100644 --- a/log/logger_test.go +++ b/log/logger_test.go @@ -50,6 +50,25 @@ func TestTerminalHandlerWithAttrs(t *testing.T) { } } +// Make sure the default json handler outputs debug log lines +func TestJSONHandler(t *testing.T) { + out := new(bytes.Buffer) + handler := JSONHandler(out) + logger := slog.New(handler) + logger.Debug("hi there") + if len(out.String()) == 0 { + t.Error("expected non-empty debug log output from default JSON Handler") + } + + out.Reset() + handler = JSONHandlerWithLevel(out, slog.LevelInfo) + logger = slog.New(handler) + logger.Debug("hi there") + if len(out.String()) != 0 { + t.Errorf("expected empty debug log output, but got: %v", out.String()) + } +} + func BenchmarkTraceLogging(b *testing.B) { SetDefault(NewLogger(NewTerminalHandler(os.Stderr, true))) b.ResetTimer() From 7aafad2233b676b7beaf56e89f82360704d669d0 Mon Sep 17 00:00:00 2001 From: Martin HS Date: Sat, 6 Apr 2024 12:22:55 +0200 Subject: [PATCH 7/7] core/vm: better error-info for vm errors (#29354) --- core/vm/errors.go | 1 + core/vm/gas_table.go | 11 +++++++++-- core/vm/gas_table_test.go | 3 ++- core/vm/interpreter.go | 7 ++++++- .../call_tracer_flat/callcode_precompiled_throw.json | 2 +- .../call_tracer_flat/nested_create_action_gas.json | 2 +- 6 files changed, 20 insertions(+), 6 deletions(-) diff --git a/core/vm/errors.go b/core/vm/errors.go index ba3261c797..e5efc952d4 100644 --- a/core/vm/errors.go +++ b/core/vm/errors.go @@ -31,6 +31,7 @@ var ( ErrContractAddressCollision = errors.New("contract address collision") ErrExecutionReverted = errors.New("execution reverted") ErrMaxCodeSizeExceeded = errors.New("max code size exceeded") + ErrMaxInitCodeSizeExceeded = errors.New("max initcode size exceeded") ErrInvalidJump = errors.New("invalid jump destination") ErrWriteProtection = errors.New("write protection") ErrReturnDataOutOfBounds = errors.New("return data out of bounds") diff --git a/core/vm/gas_table.go b/core/vm/gas_table.go index 4b141d8f9a..fd5fa14cf5 100644 --- a/core/vm/gas_table.go +++ b/core/vm/gas_table.go @@ -18,6 +18,7 @@ package vm import ( "errors" + "fmt" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" @@ -310,9 +311,12 @@ func gasCreateEip3860(evm *EVM, contract *Contract, stack *Stack, mem *Memory, m return 0, err } size, overflow := stack.Back(2).Uint64WithOverflow() - if overflow || size > params.MaxInitCodeSize { + if overflow { return 0, ErrGasUintOverflow } + if size > params.MaxInitCodeSize { + return 0, fmt.Errorf("%w: size %d", ErrMaxInitCodeSizeExceeded, size) + } // Since size <= params.MaxInitCodeSize, these multiplication cannot overflow moreGas := params.InitCodeWordGas * ((size + 31) / 32) if gas, overflow = math.SafeAdd(gas, moreGas); overflow { @@ -326,9 +330,12 @@ func gasCreate2Eip3860(evm *EVM, contract *Contract, stack *Stack, mem *Memory, return 0, err } size, overflow := stack.Back(2).Uint64WithOverflow() - if overflow || size > params.MaxInitCodeSize { + if overflow { return 0, ErrGasUintOverflow } + if size > params.MaxInitCodeSize { + return 0, fmt.Errorf("%w: size %d", ErrMaxInitCodeSizeExceeded, size) + } // Since size <= params.MaxInitCodeSize, these multiplication cannot overflow moreGas := (params.InitCodeWordGas + params.Keccak256WordGas) * ((size + 31) / 32) if gas, overflow = math.SafeAdd(gas, moreGas); overflow { diff --git a/core/vm/gas_table_test.go b/core/vm/gas_table_test.go index 4a2545b6ed..02fc94840d 100644 --- a/core/vm/gas_table_test.go +++ b/core/vm/gas_table_test.go @@ -18,6 +18,7 @@ package vm import ( "bytes" + "errors" "math" "math/big" "sort" @@ -98,7 +99,7 @@ func TestEIP2200(t *testing.T) { vmenv := NewEVM(vmctx, TxContext{}, statedb, params.AllEthashProtocolChanges, Config{ExtraEips: []int{2200}}) _, gas, err := vmenv.Call(AccountRef(common.Address{}), address, nil, tt.gaspool, new(uint256.Int)) - if err != tt.failure { + if !errors.Is(err, tt.failure) { t.Errorf("test %d: failure mismatch: have %v, want %v", i, err, tt.failure) } if used := tt.gaspool - gas; used != tt.used { diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index edf21b17d7..406927e321 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -17,6 +17,8 @@ package vm import ( + "fmt" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/tracing" @@ -255,7 +257,10 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( var dynamicCost uint64 dynamicCost, err = operation.dynamicGas(in.evm, contract, stack, mem, memorySize) cost += dynamicCost // for tracing - if err != nil || !contract.UseGas(dynamicCost, in.evm.Config.Tracer, tracing.GasChangeIgnored) { + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrOutOfGas, err) + } + if !contract.UseGas(dynamicCost, in.evm.Config.Tracer, tracing.GasChangeIgnored) { return nil, ErrOutOfGas } diff --git a/eth/tracers/internal/tracetest/testdata/call_tracer_flat/callcode_precompiled_throw.json b/eth/tracers/internal/tracetest/testdata/call_tracer_flat/callcode_precompiled_throw.json index 3c1e370f91..5e27261554 100644 --- a/eth/tracers/internal/tracetest/testdata/call_tracer_flat/callcode_precompiled_throw.json +++ b/eth/tracers/internal/tracetest/testdata/call_tracer_flat/callcode_precompiled_throw.json @@ -57,7 +57,7 @@ "gas": "0x1a034", "init": "0x36600060003760406103e8366000600060095af26001556103e8516002556104085160035500" }, - "error": "out of gas", + "error": "out of gas: not enough gas for reentrancy sentry", "traceAddress": [], "subtraces": 1, "transactionPosition": 117, diff --git a/eth/tracers/internal/tracetest/testdata/call_tracer_flat/nested_create_action_gas.json b/eth/tracers/internal/tracetest/testdata/call_tracer_flat/nested_create_action_gas.json index 132b84df36..a00ea7a93b 100644 --- a/eth/tracers/internal/tracetest/testdata/call_tracer_flat/nested_create_action_gas.json +++ b/eth/tracers/internal/tracetest/testdata/call_tracer_flat/nested_create_action_gas.json @@ -57,7 +57,7 @@ "gas": "0x19ee4", "init": "0x5a600055600060006000f0505a60015500" }, - "error": "out of gas", + "error": "out of gas: not enough gas for reentrancy sentry", "traceAddress": [], "subtraces": 1, "transactionPosition": 63,