From 7ee9a6e89f59cee21b5852f5f6ffa2bcfc05a25f Mon Sep 17 00:00:00 2001 From: Martin HS Date: Fri, 5 Apr 2024 19:29:44 +0200 Subject: [PATCH] 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 {