From da679442dbb49092a7100b9e9439d109e1da1611 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 12 Feb 2024 11:21:08 +0100 Subject: [PATCH 1/8] signer: implement blob txs sendtxargs, enable blobtx-signing --- internal/ethapi/api.go | 6 --- signer/core/apitypes/types.go | 63 ++++++++++++++++++++++++------ signer/core/apitypes/types_test.go | 61 ++++++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 18 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 752e8f9a2c70..20dceba74ba9 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -499,9 +499,6 @@ func (s *PersonalAccountAPI) SignTransaction(ctx context.Context, args Transacti if args.GasPrice == nil && (args.MaxFeePerGas == nil || args.MaxPriorityFeePerGas == 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") } @@ -1878,9 +1875,6 @@ func (s *TransactionAPI) SignTransaction(ctx context.Context, args TransactionAr 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") } diff --git a/signer/core/apitypes/types.go b/signer/core/apitypes/types.go index 6bfcd2a727b4..a0c89e3efde9 100644 --- a/signer/core/apitypes/types.go +++ b/signer/core/apitypes/types.go @@ -34,6 +34,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 +94,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,6 +119,17 @@ 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 { // Add the To-field, if specified @@ -117,15 +139,34 @@ func (args *SendTxArgs) ToTransaction() *types.Transaction { to = &dstAddr } - var input []byte - if args.Input != nil { - input = *args.Input - } else if args.Data != nil { - input = *args.Data - } - 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 +180,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 +191,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,7 +201,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(), } } return types.NewTx(data) diff --git a/signer/core/apitypes/types_test.go b/signer/core/apitypes/types_test.go index b5aa3d1e9347..fc9341da6e9e 100644 --- a/signer/core/apitypes/types_test.go +++ b/signer/core/apitypes/types_test.go @@ -16,7 +16,13 @@ package apitypes -import "testing" +import ( + "encoding/json" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" +) func TestIsPrimitive(t *testing.T) { t.Parallel() @@ -39,3 +45,56 @@ 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) + } + if have := txArgs.ToTransaction().Type(); have != tc.wantType { + t.Errorf("test %d, have type %d, want type %d", i, have, tc.wantType) + } + if have := txArgs.ToTransaction().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) + } + if have, want := txArgs.ToTransaction().Hash(), txArgs2.ToTransaction().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) + */ +} From 43f29d5f3a99779287516dc59f33978388b7aff8 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 12 Feb 2024 14:10:53 +0100 Subject: [PATCH 2/8] internal/ethapi: remove unused param --- internal/ethapi/transaction_args.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 03ffb7524f59..b7d064f85c08 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) 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 { @@ -285,7 +285,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 From 91fa8dfdace6751a20c5016488ace67a5feb5b32 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 12 Feb 2024 14:40:27 +0100 Subject: [PATCH 3/8] accounts/external: pass on blob-tx to backend signer --- accounts/external/backend.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/accounts/external/backend.go b/accounts/external/backend.go index 6f1581f9b806..721b05fbac8d 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,18 @@ 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 + // TODO verify consistency between blobs/committments/proofs + } + var res signTransactionResult if err := api.client.Call(&res, "account_signTransaction", args); err != nil { return nil, err From a39e697b52f10c922b06d57b32146f6b282ae1a7 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 12 Feb 2024 15:34:27 +0100 Subject: [PATCH 4/8] signer: enable signing of blob transactions --- internal/ethapi/api.go | 2 + signer/core/api.go | 5 +- signer/core/apitypes/types.go | 80 ++++++++++++++++++++++++++++-- signer/core/apitypes/types_test.go | 43 +++++++++++++++- signer/core/cliui.go | 8 ++- signer/fourbyte/validation.go | 5 ++ 6 files changed, 137 insertions(+), 6 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 20dceba74ba9..17b13016ca67 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1869,6 +1869,8 @@ 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") } diff --git a/signer/core/api.go b/signer/core/api.go index a32f24cb18c4..23ddcd0a2036 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 a0c89e3efde9..635e3f453216 100644 --- a/signer/core/apitypes/types.go +++ b/signer/core/apitypes/types.go @@ -28,6 +28,7 @@ import ( "strconv" "strings" + "crypto/sha256" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -131,14 +132,16 @@ func (args *SendTxArgs) data() []byte { } // 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 } - + if err := args.validateTxSidecar(); err != nil { + return nil, err + } var data types.TxData switch { case args.BlobHashes != nil: @@ -204,7 +207,78 @@ func (args *SendTxArgs) ToTransaction() *types.Transaction { 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 fc9341da6e9e..761b2ef5af2f 100644 --- a/signer/core/apitypes/types_test.go +++ b/signer/core/apitypes/types_test.go @@ -20,8 +20,11 @@ import ( "encoding/json" "testing" + "crypto/sha256" "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) { @@ -83,7 +86,9 @@ func TestTxArgs(t *testing.T) { if err := json.Unmarshal(d2, &txArgs2); err != nil { t.Fatal(err) } - if have, want := txArgs.ToTransaction().Hash(), txArgs2.ToTransaction().Hash(); have != want { + 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) } } @@ -98,3 +103,39 @@ func TestTxArgs(t *testing.T) { > eth.signTransaction(tx) */ } + +func TestBlobTxs(t *testing.T) { + blob := kzg4844.Blob{0x1} + committment, err := kzg4844.BlobToCommitment(blob) + if err != nil { + t.Fatal(err) + } + proof, err := kzg4844.ComputeBlobProof(blob, committment) + if err != nil { + t.Fatal(err) + } + + hash := kzg4844.CalcBlobHashV1(sha256.New(), &committment) + 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{committment}, + 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 b1bd3206ed3f..e04077865d5e 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 58111e8e00c8..0451bda91dc0 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 1fd10adab9befd12eb2540291241c43ff2662d45 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 12 Feb 2024 15:47:52 +0100 Subject: [PATCH 5/8] accounts/external: minor --- accounts/external/backend.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/accounts/external/backend.go b/accounts/external/backend.go index 721b05fbac8d..0de4afeabd26 100644 --- a/accounts/external/backend.go +++ b/accounts/external/backend.go @@ -244,9 +244,8 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio args.Blobs = sidecar.Blobs args.Commitments = sidecar.Commitments args.Proofs = sidecar.Proofs - // TODO verify consistency between blobs/committments/proofs } - + var res signTransactionResult if err := api.client.Call(&res, "account_signTransaction", args); err != nil { return nil, err From 75f03ce450eeec5acfeb158003f13122d01fa2e1 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 8 Mar 2024 15:41:25 +0100 Subject: [PATCH 6/8] core/types, internal/ethapi: add back blobs after signing --- core/types/transaction.go | 20 ++++++++++++++++++++ core/types/tx_blob.go | 6 ++++++ internal/ethapi/api.go | 13 +++++++++++++ 3 files changed, 39 insertions(+) diff --git a/core/types/transaction.go b/core/types/transaction.go index 7d2e9d5325a6..3208d9d2234d 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 25a85695efc9..ce1f287caaf8 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 17b13016ca67..66ee42273430 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -499,6 +499,9 @@ func (s *PersonalAccountAPI) SignTransaction(ctx context.Context, args Transacti if args.GasPrice == nil && (args.MaxFeePerGas == nil || args.MaxPriorityFeePerGas == 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") } @@ -1892,6 +1895,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{ + args.Blobs, + args.Commitments, + args.Proofs, + }) + } data, err := signed.MarshalBinary() if err != nil { return nil, err From bcd9790781b1d7979a56a28f852b0d041e9e659f Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 8 Mar 2024 16:07:26 +0100 Subject: [PATCH 7/8] internal/ethapi, signer/core/apitypes: updates to tests --- internal/ethapi/api_test.go | 7 ++----- signer/core/apitypes/types_test.go | 8 ++++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 3f69f861444c..bc6bf37a29d5 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/signer/core/apitypes/types_test.go b/signer/core/apitypes/types_test.go index 761b2ef5af2f..daffdefc2af0 100644 --- a/signer/core/apitypes/types_test.go +++ b/signer/core/apitypes/types_test.go @@ -72,10 +72,14 @@ func TestTxArgs(t *testing.T) { if err := json.Unmarshal(tc.data, &txArgs); err != nil { t.Fatal(err) } - if have := txArgs.ToTransaction().Type(); have != tc.wantType { + 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 := txArgs.ToTransaction().Hash(); have != tc.want { + if have := tx.Hash(); have != tc.want { t.Errorf("test %d: have %v, want %v", i, have, tc.want) } d2, err := json.Marshal(txArgs) From 9c2bb339aa96395df77cbb8b2c995a09e4c92557 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 9 Mar 2024 10:04:22 +0100 Subject: [PATCH 8/8] accounts, internal, signer: fix linter nits --- accounts/external/backend.go | 2 +- internal/ethapi/api.go | 6 +++--- signer/core/apitypes/types.go | 2 +- signer/core/apitypes/types_test.go | 12 +++++------- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/accounts/external/backend.go b/accounts/external/backend.go index 0de4afeabd26..0b336448fc17 100644 --- a/accounts/external/backend.go +++ b/accounts/external/backend.go @@ -245,7 +245,7 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio 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/internal/ethapi/api.go b/internal/ethapi/api.go index 93d1d4e24137..8114bdaf3e6a 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1895,9 +1895,9 @@ func (s *TransactionAPI) SignTransaction(ctx context.Context, args TransactionAr // to put back the blob(s). if args.IsEIP4844() { signed = signed.WithBlobTxSidecar(&types.BlobTxSidecar{ - args.Blobs, - args.Commitments, - args.Proofs, + Blobs: args.Blobs, + Commitments: args.Commitments, + Proofs: args.Proofs, }) } data, err := signed.MarshalBinary() diff --git a/signer/core/apitypes/types.go b/signer/core/apitypes/types.go index d966172a58b8..1aa040b890fa 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" @@ -28,7 +29,6 @@ import ( "strconv" "strings" - "crypto/sha256" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" diff --git a/signer/core/apitypes/types_test.go b/signer/core/apitypes/types_test.go index daffdefc2af0..324ff8a840d3 100644 --- a/signer/core/apitypes/types_test.go +++ b/signer/core/apitypes/types_test.go @@ -17,10 +17,10 @@ package apitypes import ( + "crypto/sha256" "encoding/json" "testing" - "crypto/sha256" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto/kzg4844" @@ -67,7 +67,6 @@ func TestTxArgs(t *testing.T) { wantType: types.DynamicFeeTxType, }, } { - var txArgs SendTxArgs if err := json.Unmarshal(tc.data, &txArgs); err != nil { t.Fatal(err) @@ -110,16 +109,16 @@ func TestTxArgs(t *testing.T) { func TestBlobTxs(t *testing.T) { blob := kzg4844.Blob{0x1} - committment, err := kzg4844.BlobToCommitment(blob) + commitment, err := kzg4844.BlobToCommitment(blob) if err != nil { t.Fatal(err) } - proof, err := kzg4844.ComputeBlobProof(blob, committment) + proof, err := kzg4844.ComputeBlobProof(blob, commitment) if err != nil { t.Fatal(err) } - hash := kzg4844.CalcBlobHashV1(sha256.New(), &committment) + hash := kzg4844.CalcBlobHashV1(sha256.New(), &commitment) b := &types.BlobTx{ ChainID: uint256.NewInt(6), Nonce: 8, @@ -131,7 +130,7 @@ func TestBlobTxs(t *testing.T) { Value: uint256.NewInt(100), Sidecar: &types.BlobTxSidecar{ Blobs: []kzg4844.Blob{blob}, - Commitments: []kzg4844.Commitment{committment}, + Commitments: []kzg4844.Commitment{commitment}, Proofs: []kzg4844.Proof{proof}, }, } @@ -141,5 +140,4 @@ func TestBlobTxs(t *testing.T) { t.Fatal(err) } t.Logf("tx %v", string(data)) - }