From f07dc434f550262e13a6bfc3b3cc0f6b942f3e52 Mon Sep 17 00:00:00 2001 From: otherview Date: Mon, 16 Sep 2024 15:54:10 +0100 Subject: [PATCH 1/8] Enhance Tx Builder --- api/accounts/accounts_test.go | 11 +++----- api/blocks/blocks_test.go | 11 +++----- api/debug/debug_test.go | 22 +++++----------- api/subscriptions/block_reader_test.go | 12 +++------ api/subscriptions/pending_tx_test.go | 12 +++------ api/subscriptions/types_test.go | 23 +++++----------- api/transactions/transactions_test.go | 36 ++++++++------------------ cmd/thor/node/tx_stash_test.go | 6 ++--- cmd/thor/solo/solo.go | 9 ++----- consensus/consensus_test.go | 6 ++--- packer/packer_test.go | 6 ++--- runtime/resolved_tx_test.go | 6 ++--- tx/builder.go | 36 ++++++++++++++++++++++++++ txpool/tx_object_test.go | 24 ++++++----------- 14 files changed, 95 insertions(+), 125 deletions(-) diff --git a/api/accounts/accounts_test.go b/api/accounts/accounts_test.go index 16f08e252..76c6e5240 100644 --- a/api/accounts/accounts_test.go +++ b/api/accounts/accounts_test.go @@ -19,9 +19,9 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/common/math" - "github.com/ethereum/go-ethereum/crypto" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ABI "github.com/vechain/thor/v2/abi" "github.com/vechain/thor/v2/api/accounts" "github.com/vechain/thor/v2/block" @@ -293,12 +293,9 @@ func buildTxWithClauses(t *testing.T, chaiTag byte, clauses ...*tx.Clause) *tx.T builder.Clause(c) } - transaction := builder.Build() - sig, err := crypto.Sign(transaction.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - return transaction.WithSignature(sig) + transaction, err := builder.BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) + return transaction } func packTx(repo *chain.Repository, stater *state.Stater, transaction *tx.Transaction, t *testing.T) { diff --git a/api/blocks/blocks_test.go b/api/blocks/blocks_test.go index 0ad5350b5..63dec2d64 100644 --- a/api/blocks/blocks_test.go +++ b/api/blocks/blocks_test.go @@ -17,7 +17,6 @@ import ( "testing" "time" - "github.com/ethereum/go-ethereum/crypto" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -171,7 +170,7 @@ func initBlockServer(t *testing.T) { repo, _ := chain.NewRepository(db, b) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - tx := new(tx.Builder). + tx, err := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -179,13 +178,9 @@ func initBlockServer(t *testing.T) { Nonce(1). Clause(cla). BlockRef(tx.NewBlockRef(0)). - Build() + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) - sig, err := crypto.Sign(tx.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - tx = tx.WithSignature(sig) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) flow, err := packer.Schedule(sum, uint64(time.Now().Unix())) diff --git a/api/debug/debug_test.go b/api/debug/debug_test.go index 3c61dfcab..40cbd42b5 100644 --- a/api/debug/debug_test.go +++ b/api/debug/debug_test.go @@ -20,9 +20,9 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/common/math" - "github.com/ethereum/go-ethereum/crypto" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/builtin" "github.com/vechain/thor/v2/chain" @@ -527,20 +527,16 @@ func initDebugServer(t *testing.T) { // Adding an empty clause transaction to the block to cover the case of // scanning multiple txs before getting the right one - noClausesTx := new(tx.Builder). + noClausesTx, err := new(tx.Builder). ChainTag(repo.ChainTag()). Expiration(10). Gas(21000). - Build() - sig, err := crypto.Sign(noClausesTx.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - noClausesTx = noClausesTx.WithSignature(sig) + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) cla2 := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - transaction = new(tx.Builder). + transaction, err = new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -549,13 +545,9 @@ func initDebugServer(t *testing.T) { Clause(cla). Clause(cla2). BlockRef(tx.NewBlockRef(0)). - Build() + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) - sig, err = crypto.Sign(transaction.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - transaction = transaction.WithSignature(sig) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) flow, err := packer.Schedule(sum, uint64(time.Now().Unix())) diff --git a/api/subscriptions/block_reader_test.go b/api/subscriptions/block_reader_test.go index f6db221e5..13feb9901 100644 --- a/api/subscriptions/block_reader_test.go +++ b/api/subscriptions/block_reader_test.go @@ -10,8 +10,8 @@ import ( "testing" "time" - "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -77,7 +77,7 @@ func initChain(t *testing.T) (*chain.Repository, []*block.Block, *txpool.TxPool) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - tr := new(tx.Builder). + tr, err := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -85,13 +85,9 @@ func initChain(t *testing.T) (*chain.Repository, []*block.Block, *txpool.TxPool) Nonce(1). Clause(cla). BlockRef(tx.NewBlockRef(0)). - Build() + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) - sig, err := crypto.Sign(tr.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - tr = tr.WithSignature(sig) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) flow, err := packer.Schedule(sum, uint64(time.Now().Unix())) diff --git a/api/subscriptions/pending_tx_test.go b/api/subscriptions/pending_tx_test.go index 629b96e0c..6eef399fc 100644 --- a/api/subscriptions/pending_tx_test.go +++ b/api/subscriptions/pending_tx_test.go @@ -10,8 +10,8 @@ import ( "testing" "time" - "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -132,7 +132,7 @@ func addNewBlock(repo *chain.Repository, stater *state.Stater, b0 *block.Block, func createTx(t *testing.T, repo *chain.Repository, addressNumber uint) *tx.Transaction { addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - tx := new(tx.Builder). + tx, err := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -140,12 +140,8 @@ func createTx(t *testing.T, repo *chain.Repository, addressNumber uint) *tx.Tran Nonce(1). Clause(cla). BlockRef(tx.NewBlockRef(0)). - Build() - sig, err := crypto.Sign(tx.SigningHash().Bytes(), genesis.DevAccounts()[addressNumber].PrivateKey) - if err != nil { - t.Fatal(err) - } - tx = tx.WithSignature(sig) + BuildAndSign(genesis.DevAccounts()[addressNumber].PrivateKey) + require.NoError(t, err) return tx } diff --git a/api/subscriptions/types_test.go b/api/subscriptions/types_test.go index 05492dd98..7cb498a28 100644 --- a/api/subscriptions/types_test.go +++ b/api/subscriptions/types_test.go @@ -14,6 +14,7 @@ import ( "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -95,20 +96,15 @@ func TestConvertTransfer(t *testing.T) { repo, _ := chain.NewRepository(db, b) // New tx - transaction := new(tx.Builder). + transaction, err := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). Gas(21000). Nonce(1). BlockRef(tx.NewBlockRef(0)). - Build() - - sig, err := crypto.Sign(transaction.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - transaction = transaction.WithSignature(sig) + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) // New block blk := new(block.Builder). @@ -187,20 +183,15 @@ func TestConvertEvent(t *testing.T) { repo, _ := chain.NewRepository(db, b) // New tx - transaction := new(tx.Builder). + transaction, err := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). Gas(21000). Nonce(1). BlockRef(tx.NewBlockRef(0)). - Build() - - sig, err := crypto.Sign(transaction.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - transaction = transaction.WithSignature(sig) + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) // New block blk := new(block.Builder). diff --git a/api/transactions/transactions_test.go b/api/transactions/transactions_test.go index 2d86d13e3..ca001531a 100644 --- a/api/transactions/transactions_test.go +++ b/api/transactions/transactions_test.go @@ -18,10 +18,10 @@ import ( "time" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rlp" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/api/transactions" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -112,17 +112,14 @@ func sendTx(t *testing.T) { var expiration = uint32(10) var gas = uint64(21000) - tx := new(tx.Builder). + tx, err := new(tx.Builder). BlockRef(blockRef). ChainTag(chainTag). Expiration(expiration). Gas(gas). - Build() - sig, err := crypto.Sign(tx.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - tx = tx.WithSignature(sig) + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) + rlpTx, err := rlp.EncodeToBytes(tx) if err != nil { t.Fatal(err) @@ -287,7 +284,7 @@ func initTransactionServer(t *testing.T) { repo, _ = chain.NewRepository(db, b) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - transaction = new(tx.Builder). + transaction, err = new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -295,27 +292,16 @@ func initTransactionServer(t *testing.T) { Nonce(1). Clause(cla). BlockRef(tx.NewBlockRef(0)). - Build() + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) - mempoolTx = new(tx.Builder). + mempoolTx, err = new(tx.Builder). ChainTag(repo.ChainTag()). Expiration(10). Gas(21000). Nonce(1). - Build() - - sig, err := crypto.Sign(transaction.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - - sig2, err := crypto.Sign(mempoolTx.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - if err != nil { - t.Fatal(err) - } - - transaction = transaction.WithSignature(sig) - mempoolTx = mempoolTx.WithSignature(sig2) + BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + require.NoError(t, err) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/cmd/thor/node/tx_stash_test.go b/cmd/thor/node/tx_stash_test.go index a573b682a..274ef7981 100644 --- a/cmd/thor/node/tx_stash_test.go +++ b/cmd/thor/node/tx_stash_test.go @@ -11,7 +11,6 @@ import ( "sort" "testing" - "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/storage" @@ -20,9 +19,8 @@ import ( ) func newTx() *tx.Transaction { - tx := new(tx.Builder).Nonce(rand.Uint64()).Build() // nolint:gosec - sig, _ := crypto.Sign(tx.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - return tx.WithSignature(sig) + tx, _ := new(tx.Builder).Nonce(rand.Uint64()).BuildAndSign(genesis.DevAccounts()[0].PrivateKey) // nolint:gosec + return tx } func TestTxStash(t *testing.T) { diff --git a/cmd/thor/solo/solo.go b/cmd/thor/solo/solo.go index 43dd644a1..4bcb61f8c 100644 --- a/cmd/thor/solo/solo.go +++ b/cmd/thor/solo/solo.go @@ -15,7 +15,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/common/mclock" - "github.com/ethereum/go-ethereum/crypto" "github.com/pkg/errors" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/builtin" @@ -261,14 +260,10 @@ func (s *Solo) newTx(clauses []*tx.Clause, from genesis.DevAccount) (*tx.Transac builder.Clause(c) } - newTx := builder.BlockRef(tx.NewBlockRef(0)). + return builder.BlockRef(tx.NewBlockRef(0)). Expiration(math.MaxUint32). Nonce(rand.Uint64()). // nolint:gosec DependsOn(nil). Gas(1_000_000). - Build() - - sig, err := crypto.Sign(newTx.SigningHash().Bytes(), from.PrivateKey) - - return newTx.WithSignature(sig), err + BuildAndSign(from.PrivateKey) } diff --git a/consensus/consensus_test.go b/consensus/consensus_test.go index 10f01e04e..9d726bd74 100644 --- a/consensus/consensus_test.go +++ b/consensus/consensus_test.go @@ -14,6 +14,7 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/builtin" "github.com/vechain/thor/v2/chain" @@ -577,9 +578,8 @@ func TestValidateBlockBody(t *testing.T) { { "TxOriginBlocked", func(t *testing.T) { thor.MockBlocklist([]string{genesis.DevAccounts()[9].Address.String()}) - tx := txBuilder(tc.tag).Build() - sig, _ := crypto.Sign(tx.SigningHash().Bytes(), genesis.DevAccounts()[9].PrivateKey) - tx = tx.WithSignature(sig) + tx, err := txBuilder(tc.tag).BuildAndSign(genesis.DevAccounts()[9].PrivateKey) + require.NoError(t, err) blk, err := tc.sign( tc.builder(tc.original.Header()).Transaction(tx), diff --git a/packer/packer_test.go b/packer/packer_test.go index 4898804e4..d0f8ac333 100644 --- a/packer/packer_test.go +++ b/packer/packer_test.go @@ -50,13 +50,11 @@ func (ti *txIterator) Next() *tx.Transaction { data, _ := method.EncodeInput(a1.Address, big.NewInt(1)) - tx := new(tx.Builder). + tx, _ := new(tx.Builder). ChainTag(ti.chainTag). Clause(tx.NewClause(&builtin.Energy.Address).WithData(data)). - Gas(300000).GasPriceCoef(0).Nonce(nonce).Expiration(math.MaxUint32).Build() + Gas(300000).GasPriceCoef(0).Nonce(nonce).Expiration(math.MaxUint32).BuildAndSign(a0.PrivateKey) nonce++ - sig, _ := crypto.Sign(tx.SigningHash().Bytes(), a0.PrivateKey) - tx = tx.WithSignature(sig) return tx } diff --git a/runtime/resolved_tx_test.go b/runtime/resolved_tx_test.go index 0588f1356..97c31d32c 100644 --- a/runtime/resolved_tx_test.go +++ b/runtime/resolved_tx_test.go @@ -12,7 +12,6 @@ import ( "testing" "github.com/ethereum/go-ethereum/common/math" - "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/secp256k1" "github.com/stretchr/testify/assert" "github.com/vechain/thor/v2/builtin" @@ -186,7 +185,6 @@ func txBuilder(tag byte) *tx.Builder { } func txSign(builder *tx.Builder) *tx.Transaction { - transaction := builder.Build() - sig, _ := crypto.Sign(transaction.SigningHash().Bytes(), genesis.DevAccounts()[0].PrivateKey) - return transaction.WithSignature(sig) + transaction, _ := builder.BuildAndSign(genesis.DevAccounts()[0].PrivateKey) + return transaction } diff --git a/tx/builder.go b/tx/builder.go index 64f98868f..b7e25df9c 100644 --- a/tx/builder.go +++ b/tx/builder.go @@ -6,8 +6,11 @@ package tx import ( + "crypto/ecdsa" "encoding/binary" + "fmt" + "github.com/ethereum/go-ethereum/crypto" "github.com/vechain/thor/v2/thor" ) @@ -80,3 +83,36 @@ func (b *Builder) Build() *Transaction { tx := Transaction{body: b.body} return &tx } + +// BuildAndSign builds tx object and signs it using the provided key. +func (b *Builder) BuildAndSign(privateKey *ecdsa.PrivateKey) (*Transaction, error) { + tx := b.Build() + sig, err := crypto.Sign(tx.SigningHash().Bytes(), privateKey) + if err != nil { + return nil, fmt.Errorf("unable to sign transaction: %w", err) + } + + return tx.WithSignature(sig), nil +} + +func (b *Builder) BuildAndSignWithDelegator(privateKey *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { + tx := b.Build() + sig, err := crypto.Sign(tx.SigningHash().Bytes(), privateKey) + if err != nil { + return nil, fmt.Errorf("unable to sign transaction: %w", err) + } + + pub, err := crypto.SigToPub(tx.SigningHash().Bytes(), sig[:65]) + if err != nil { + return nil, fmt.Errorf("unable to recover address from signature: %w", err) + } + origin := thor.Address(crypto.PubkeyToAddress(*pub)) + + dSig, err := crypto.Sign(tx.DelegatorSigningHash(origin).Bytes(), delegatorPK) + if err != nil { + return nil, fmt.Errorf("unable to delegator sign transaction: %w", err) + } + sig = append(sig, dSig...) + + return tx.WithSignature(sig), nil +} diff --git a/txpool/tx_object_test.go b/txpool/tx_object_test.go index fd384fdcb..9deca6edb 100644 --- a/txpool/tx_object_test.go +++ b/txpool/tx_object_test.go @@ -11,7 +11,6 @@ import ( "math/rand" "testing" - "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" @@ -29,25 +28,21 @@ func newChainRepo(db *muxdb.MuxDB) *chain.Repository { return repo } -func signTx(tx *tx.Transaction, acc genesis.DevAccount) *tx.Transaction { - sig, _ := crypto.Sign(tx.SigningHash().Bytes(), acc.PrivateKey) - return tx.WithSignature(sig) -} - func newTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx.BlockRef, expiration uint32, dependsOn *thor.Bytes32, features tx.Features, from genesis.DevAccount) *tx.Transaction { builder := new(tx.Builder).ChainTag(chainTag) for _, c := range clauses { builder.Clause(c) } - tx := builder.BlockRef(blockRef). + tx, _ := builder.BlockRef(blockRef). Expiration(expiration). Nonce(rand.Uint64()). // nolint:gosec DependsOn(dependsOn). Features(features). - Gas(gas).Build() + Gas(gas). + BuildAndSign(from.PrivateKey) - return signTx(tx, from) + return tx } func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx.BlockRef, expiration uint32, dependsOn *thor.Bytes32, from genesis.DevAccount, delegator genesis.DevAccount) *tx.Transaction { @@ -59,18 +54,15 @@ func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx var features tx.Features features.SetDelegated(true) - tx := builder.BlockRef(blockRef). + tx, _ := builder.BlockRef(blockRef). Expiration(expiration). Nonce(rand.Uint64()). // nolint:gosec DependsOn(dependsOn). Features(features). - Gas(gas).Build() - - sig, _ := crypto.Sign(tx.SigningHash().Bytes(), from.PrivateKey) - dSig, _ := crypto.Sign(tx.DelegatorSigningHash(from.Address).Bytes(), delegator.PrivateKey) + Gas(gas). + BuildAndSignWithDelegator(from.PrivateKey, delegator.PrivateKey) - sig = append(sig, dSig...) - return tx.WithSignature(sig) + return tx } func SetupTest() (genesis.DevAccount, *chain.Repository, *block.Block, *state.State) { From fb481d25728da3cf5e0a3eaedc5dad1879aa86f2 Mon Sep 17 00:00:00 2001 From: otherview Date: Tue, 24 Sep 2024 14:08:55 +0100 Subject: [PATCH 2/8] changing the builder to a tx.signer --- api/accounts/accounts_test.go | 13 ++--- api/blocks/blocks_test.go | 24 ++++---- api/debug/debug_test.go | 13 ++--- api/subscriptions/block_reader_test.go | 7 +-- api/subscriptions/pending_tx_test.go | 32 +++++------ api/subscriptions/types_test.go | 27 ++++----- api/transactions/transactions_test.go | 35 ++++++------ cmd/thor/node/tx_stash_test.go | 7 ++- cmd/thor/solo/solo.go | 5 +- consensus/consensus_test.go | 7 +-- packer/packer_test.go | 7 ++- runtime/resolved_tx_test.go | 4 +- tx/builder.go | 36 ------------ tx/signer.go | 79 ++++++++++++++++++++++++++ txpool/tx_object_test.go | 19 ++++--- 15 files changed, 184 insertions(+), 131 deletions(-) create mode 100644 tx/signer.go diff --git a/api/accounts/accounts_test.go b/api/accounts/accounts_test.go index 76c6e5240..70590c318 100644 --- a/api/accounts/accounts_test.go +++ b/api/accounts/accounts_test.go @@ -21,7 +21,6 @@ import ( "github.com/ethereum/go-ethereum/common/math" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ABI "github.com/vechain/thor/v2/abi" "github.com/vechain/thor/v2/api/accounts" "github.com/vechain/thor/v2/block" @@ -262,7 +261,7 @@ func initAccountServer(t *testing.T) { repo, _ := chain.NewRepository(db, b) claTransfer := tx.NewClause(&addr).WithValue(value) claDeploy := tx.NewClause(nil).WithData(bytecode) - transaction := buildTxWithClauses(t, repo.ChainTag(), claTransfer, claDeploy) + transaction := buildTxWithClauses(repo.ChainTag(), claTransfer, claDeploy) contractAddr = thor.CreateContractAddress(transaction.ID(), 1, 0) packTx(repo, stater, transaction, t) @@ -274,7 +273,7 @@ func initAccountServer(t *testing.T) { t.Fatal(err) } claCall := tx.NewClause(&contractAddr).WithData(input) - transactionCall := buildTxWithClauses(t, repo.ChainTag(), claCall) + transactionCall := buildTxWithClauses(repo.ChainTag(), claCall) packTx(repo, stater, transactionCall, t) router := mux.NewRouter() @@ -284,7 +283,7 @@ func initAccountServer(t *testing.T) { ts = httptest.NewServer(router) } -func buildTxWithClauses(t *testing.T, chaiTag byte, clauses ...*tx.Clause) *tx.Transaction { +func buildTxWithClauses(chaiTag byte, clauses ...*tx.Clause) *tx.Transaction { builder := new(tx.Builder). ChainTag(chaiTag). Expiration(10). @@ -293,9 +292,9 @@ func buildTxWithClauses(t *testing.T, chaiTag byte, clauses ...*tx.Clause) *tx.T builder.Clause(c) } - transaction, err := builder.BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) - return transaction + trx := builder.Build() + + return tx.MustSignTx(trx, genesis.DevAccounts()[0].PrivateKey) } func packTx(repo *chain.Repository, stater *state.Stater, transaction *tx.Transaction, t *testing.T) { diff --git a/api/blocks/blocks_test.go b/api/blocks/blocks_test.go index 63dec2d64..dbaf8f0b6 100644 --- a/api/blocks/blocks_test.go +++ b/api/blocks/blocks_test.go @@ -170,16 +170,18 @@ func initBlockServer(t *testing.T) { repo, _ := chain.NewRepository(db, b) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - tx, err := new(tx.Builder). - ChainTag(repo.ChainTag()). - GasPriceCoef(1). - Expiration(10). - Gas(21000). - Nonce(1). - Clause(cla). - BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + trx := tx.MustSignTx( + new(tx.Builder). + ChainTag(repo.ChainTag()). + GasPriceCoef(1). + Expiration(10). + Gas(21000). + Nonce(1). + Clause(cla). + BlockRef(tx.NewBlockRef(0)). + Build(), + genesis.DevAccounts()[0].PrivateKey, + ) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) @@ -187,7 +189,7 @@ func initBlockServer(t *testing.T) { if err != nil { t.Fatal(err) } - err = flow.Adopt(tx) + err = flow.Adopt(trx) if err != nil { t.Fatal(err) } diff --git a/api/debug/debug_test.go b/api/debug/debug_test.go index 40cbd42b5..3b7365d2c 100644 --- a/api/debug/debug_test.go +++ b/api/debug/debug_test.go @@ -22,7 +22,6 @@ import ( "github.com/ethereum/go-ethereum/common/math" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/builtin" "github.com/vechain/thor/v2/chain" @@ -527,16 +526,16 @@ func initDebugServer(t *testing.T) { // Adding an empty clause transaction to the block to cover the case of // scanning multiple txs before getting the right one - noClausesTx, err := new(tx.Builder). + noClausesTx := new(tx.Builder). ChainTag(repo.ChainTag()). Expiration(10). Gas(21000). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + Build() + noClausesTx = tx.MustSignTx(noClausesTx, genesis.DevAccounts()[0].PrivateKey) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) cla2 := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - transaction, err = new(tx.Builder). + transaction = new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -545,8 +544,8 @@ func initDebugServer(t *testing.T) { Clause(cla). Clause(cla2). BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + Build() + transaction = tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/api/subscriptions/block_reader_test.go b/api/subscriptions/block_reader_test.go index 13feb9901..51bde8f97 100644 --- a/api/subscriptions/block_reader_test.go +++ b/api/subscriptions/block_reader_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -77,7 +76,7 @@ func initChain(t *testing.T) (*chain.Repository, []*block.Block, *txpool.TxPool) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - tr, err := new(tx.Builder). + tr := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -85,8 +84,8 @@ func initChain(t *testing.T) (*chain.Repository, []*block.Block, *txpool.TxPool) Nonce(1). Clause(cla). BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + Build() + tr = tx.MustSignTx(tr, genesis.DevAccounts()[0].PrivateKey) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/api/subscriptions/pending_tx_test.go b/api/subscriptions/pending_tx_test.go index 6eef399fc..aa8314fa7 100644 --- a/api/subscriptions/pending_tx_test.go +++ b/api/subscriptions/pending_tx_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -77,7 +76,7 @@ func TestPendingTx_DispatchLoop(t *testing.T) { p.Subscribe(txCh) // Add a new tx to the mempool - transaction := createTx(t, repo, 0) + transaction := createTx(repo, 0) txPool.AddLocal(transaction) // Start the dispatch loop @@ -95,7 +94,7 @@ func TestPendingTx_DispatchLoop(t *testing.T) { p.Unsubscribe(txCh) // Add another tx to the mempool - tx2 := createTx(t, repo, 1) + tx2 := createTx(repo, 1) txPool.AddLocal(tx2) // Assert that the channel did not receive the second transaction @@ -129,19 +128,20 @@ func addNewBlock(repo *chain.Repository, stater *state.Stater, b0 *block.Block, } } -func createTx(t *testing.T, repo *chain.Repository, addressNumber uint) *tx.Transaction { +func createTx(repo *chain.Repository, addressNumber uint) *tx.Transaction { addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - tx, err := new(tx.Builder). - ChainTag(repo.ChainTag()). - GasPriceCoef(1). - Expiration(10). - Gas(21000). - Nonce(1). - Clause(cla). - BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[addressNumber].PrivateKey) - require.NoError(t, err) - - return tx + + return tx.MustSignTx( + new(tx.Builder). + ChainTag(repo.ChainTag()). + GasPriceCoef(1). + Expiration(10). + Gas(21000). + Nonce(1). + Clause(cla). + BlockRef(tx.NewBlockRef(0)). + Build(), + genesis.DevAccounts()[addressNumber].PrivateKey, + ) } diff --git a/api/subscriptions/types_test.go b/api/subscriptions/types_test.go index 7cb498a28..8aee6b3b2 100644 --- a/api/subscriptions/types_test.go +++ b/api/subscriptions/types_test.go @@ -14,7 +14,6 @@ import ( "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -96,15 +95,15 @@ func TestConvertTransfer(t *testing.T) { repo, _ := chain.NewRepository(db, b) // New tx - transaction, err := new(tx.Builder). + transaction := new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). Gas(21000). Nonce(1). BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + Build() + transaction = tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) // New block blk := new(block.Builder). @@ -183,15 +182,17 @@ func TestConvertEvent(t *testing.T) { repo, _ := chain.NewRepository(db, b) // New tx - transaction, err := new(tx.Builder). - ChainTag(repo.ChainTag()). - GasPriceCoef(1). - Expiration(10). - Gas(21000). - Nonce(1). - BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + transaction := tx.MustSignTx( + new(tx.Builder). + ChainTag(repo.ChainTag()). + GasPriceCoef(1). + Expiration(10). + Gas(21000). + Nonce(1). + BlockRef(tx.NewBlockRef(0)). + Build(), + genesis.DevAccounts()[0].PrivateKey, + ) // New block blk := new(block.Builder). diff --git a/api/transactions/transactions_test.go b/api/transactions/transactions_test.go index ca001531a..9ec885762 100644 --- a/api/transactions/transactions_test.go +++ b/api/transactions/transactions_test.go @@ -21,7 +21,6 @@ import ( "github.com/ethereum/go-ethereum/rlp" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/api/transactions" "github.com/vechain/thor/v2/chain" "github.com/vechain/thor/v2/genesis" @@ -112,15 +111,17 @@ func sendTx(t *testing.T) { var expiration = uint32(10) var gas = uint64(21000) - tx, err := new(tx.Builder). - BlockRef(blockRef). - ChainTag(chainTag). - Expiration(expiration). - Gas(gas). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) - - rlpTx, err := rlp.EncodeToBytes(tx) + trx := tx.MustSignTx( + new(tx.Builder). + BlockRef(blockRef). + ChainTag(chainTag). + Expiration(expiration). + Gas(gas). + Build(), + genesis.DevAccounts()[0].PrivateKey, + ) + + rlpTx, err := rlp.EncodeToBytes(trx) if err != nil { t.Fatal(err) } @@ -130,7 +131,7 @@ func sendTx(t *testing.T) { if err = json.Unmarshal(res, &txObj); err != nil { t.Fatal(err) } - assert.Equal(t, tx.ID().String(), txObj["id"], "should be the same transaction id") + assert.Equal(t, trx.ID().String(), txObj["id"], "should be the same transaction id") } func getTxWithBadID(t *testing.T) { @@ -284,7 +285,7 @@ func initTransactionServer(t *testing.T) { repo, _ = chain.NewRepository(db, b) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - transaction, err = new(tx.Builder). + transaction = new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). Expiration(10). @@ -292,16 +293,16 @@ func initTransactionServer(t *testing.T) { Nonce(1). Clause(cla). BlockRef(tx.NewBlockRef(0)). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + Build() + transaction = tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) - mempoolTx, err = new(tx.Builder). + mempoolTx = new(tx.Builder). ChainTag(repo.ChainTag()). Expiration(10). Gas(21000). Nonce(1). - BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - require.NoError(t, err) + Build() + mempoolTx = tx.MustSignTx(mempoolTx, genesis.DevAccounts()[0].PrivateKey) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/cmd/thor/node/tx_stash_test.go b/cmd/thor/node/tx_stash_test.go index 274ef7981..c236a9341 100644 --- a/cmd/thor/node/tx_stash_test.go +++ b/cmd/thor/node/tx_stash_test.go @@ -19,8 +19,11 @@ import ( ) func newTx() *tx.Transaction { - tx, _ := new(tx.Builder).Nonce(rand.Uint64()).BuildAndSign(genesis.DevAccounts()[0].PrivateKey) // nolint:gosec - return tx + return tx.MustSignTx( + new(tx.Builder). + Nonce(rand.Uint64()).Build(), // nolint:gosec, + genesis.DevAccounts()[0].PrivateKey, + ) } func TestTxStash(t *testing.T) { diff --git a/cmd/thor/solo/solo.go b/cmd/thor/solo/solo.go index c6ef10800..0d00cefc5 100644 --- a/cmd/thor/solo/solo.go +++ b/cmd/thor/solo/solo.go @@ -260,10 +260,11 @@ func (s *Solo) newTx(clauses []*tx.Clause, from genesis.DevAccount) (*tx.Transac builder.Clause(c) } - return builder.BlockRef(tx.NewBlockRef(0)). + trx := builder.BlockRef(tx.NewBlockRef(0)). Expiration(math.MaxUint32). Nonce(rand.Uint64()). // #nosec DependsOn(nil). Gas(1_000_000). - BuildAndSign(from.PrivateKey) + Build() + return tx.SignTx(trx, from.PrivateKey) } diff --git a/consensus/consensus_test.go b/consensus/consensus_test.go index 9d726bd74..73cead551 100644 --- a/consensus/consensus_test.go +++ b/consensus/consensus_test.go @@ -14,7 +14,6 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/vechain/thor/v2/block" "github.com/vechain/thor/v2/builtin" "github.com/vechain/thor/v2/chain" @@ -578,11 +577,11 @@ func TestValidateBlockBody(t *testing.T) { { "TxOriginBlocked", func(t *testing.T) { thor.MockBlocklist([]string{genesis.DevAccounts()[9].Address.String()}) - tx, err := txBuilder(tc.tag).BuildAndSign(genesis.DevAccounts()[9].PrivateKey) - require.NoError(t, err) + trx := txBuilder(tc.tag).Build() + trx = tx.MustSignTx(trx, genesis.DevAccounts()[9].PrivateKey) blk, err := tc.sign( - tc.builder(tc.original.Header()).Transaction(tx), + tc.builder(tc.original.Header()).Transaction(trx), ) if err != nil { t.Fatal(err) diff --git a/packer/packer_test.go b/packer/packer_test.go index d0f8ac333..c6829700d 100644 --- a/packer/packer_test.go +++ b/packer/packer_test.go @@ -50,13 +50,14 @@ func (ti *txIterator) Next() *tx.Transaction { data, _ := method.EncodeInput(a1.Address, big.NewInt(1)) - tx, _ := new(tx.Builder). + trx := new(tx.Builder). ChainTag(ti.chainTag). Clause(tx.NewClause(&builtin.Energy.Address).WithData(data)). - Gas(300000).GasPriceCoef(0).Nonce(nonce).Expiration(math.MaxUint32).BuildAndSign(a0.PrivateKey) + Gas(300000).GasPriceCoef(0).Nonce(nonce).Expiration(math.MaxUint32).Build() + trx = tx.MustSignTx(trx, a0.PrivateKey) nonce++ - return tx + return trx } func (ti *txIterator) OnProcessed(_ thor.Bytes32, _ error) { diff --git a/runtime/resolved_tx_test.go b/runtime/resolved_tx_test.go index 97c31d32c..c0c521354 100644 --- a/runtime/resolved_tx_test.go +++ b/runtime/resolved_tx_test.go @@ -185,6 +185,6 @@ func txBuilder(tag byte) *tx.Builder { } func txSign(builder *tx.Builder) *tx.Transaction { - transaction, _ := builder.BuildAndSign(genesis.DevAccounts()[0].PrivateKey) - return transaction + transaction := builder.Build() + return tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) } diff --git a/tx/builder.go b/tx/builder.go index b7e25df9c..64f98868f 100644 --- a/tx/builder.go +++ b/tx/builder.go @@ -6,11 +6,8 @@ package tx import ( - "crypto/ecdsa" "encoding/binary" - "fmt" - "github.com/ethereum/go-ethereum/crypto" "github.com/vechain/thor/v2/thor" ) @@ -83,36 +80,3 @@ func (b *Builder) Build() *Transaction { tx := Transaction{body: b.body} return &tx } - -// BuildAndSign builds tx object and signs it using the provided key. -func (b *Builder) BuildAndSign(privateKey *ecdsa.PrivateKey) (*Transaction, error) { - tx := b.Build() - sig, err := crypto.Sign(tx.SigningHash().Bytes(), privateKey) - if err != nil { - return nil, fmt.Errorf("unable to sign transaction: %w", err) - } - - return tx.WithSignature(sig), nil -} - -func (b *Builder) BuildAndSignWithDelegator(privateKey *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { - tx := b.Build() - sig, err := crypto.Sign(tx.SigningHash().Bytes(), privateKey) - if err != nil { - return nil, fmt.Errorf("unable to sign transaction: %w", err) - } - - pub, err := crypto.SigToPub(tx.SigningHash().Bytes(), sig[:65]) - if err != nil { - return nil, fmt.Errorf("unable to recover address from signature: %w", err) - } - origin := thor.Address(crypto.PubkeyToAddress(*pub)) - - dSig, err := crypto.Sign(tx.DelegatorSigningHash(origin).Bytes(), delegatorPK) - if err != nil { - return nil, fmt.Errorf("unable to delegator sign transaction: %w", err) - } - sig = append(sig, dSig...) - - return tx.WithSignature(sig), nil -} diff --git a/tx/signer.go b/tx/signer.go new file mode 100644 index 000000000..913c0b21d --- /dev/null +++ b/tx/signer.go @@ -0,0 +1,79 @@ +package tx + +import ( + "crypto/ecdsa" + "fmt" + + "github.com/ethereum/go-ethereum/crypto" + "github.com/vechain/thor/v2/thor" +) + +// SignFunction is a type alias for a function that signs a hash using a private key. +// It returns the signature and an error if any occurs. +type SignFunction func(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) + +// MustSignTx signs a transaction with the provided private key using the default signing function. +// It panics if the signing process fails. +func MustSignTx(tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { + return MustCustomSignTx(crypto.Sign, tx, pk) +} + +// MustCustomSignTx signs a transaction using a custom signing function and private key. +// It panics if the signing process fails. +func MustCustomSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { + trx, err := CustomSignTx(sign, tx, pk) + if err != nil { + panic(err) + } + return trx +} + +// SignTx signs a transaction with the provided private key using the default signing function. +// It returns the signed transaction or an error if the signing fails. +func SignTx(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + return CustomSignTx(crypto.Sign, tx, pk) +} + +// CustomSignTx signs a transaction using a custom signing function and private key. +// It returns the signed transaction or an error if the signing fails. +func CustomSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + // Generate the signature for the transaction's signing hash. + sig, err := sign(tx.SigningHash().Bytes(), pk) + if err != nil { + return nil, fmt.Errorf("unable to sign transaction: %w", err) + } + + // Attach the signature to the transaction. + return tx.WithSignature(sig), nil +} + +// DelegatorSignTx signs a transaction as a delegator with the provided private key using the default signing function. +// It returns the signed transaction or an error if the signing fails. +func DelegatorSignTx(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + return CustomDelegatorSignTx(crypto.Sign, tx, pk) +} + +// CustomDelegatorSignTx signs a transaction as a delegator using a custom signing function and private key. +// It returns the signed transaction or an error if the signing fails. +func CustomDelegatorSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + // Extract the public key from the existing transaction signature. + pub, err := crypto.SigToPub(tx.SigningHash().Bytes(), tx.Signature()) + if err != nil { + return nil, fmt.Errorf("unable to extract public key from signature: %w", err) + } + + // Convert the public key to the corresponding address. + origin := thor.Address(crypto.PubkeyToAddress(*pub)) + + // Generate the delegator's signature using the delegator signing hash. + dSig, err := sign(tx.DelegatorSigningHash(origin).Bytes(), pk) + if err != nil { + return nil, fmt.Errorf("unable to delegator sign transaction: %w", err) + } + + // Append the delegator's signature to the existing transaction signature. + sig := append(tx.Signature(), dSig...) + + // Attach the combined signature to the transaction. + return tx.WithSignature(sig), nil +} diff --git a/txpool/tx_object_test.go b/txpool/tx_object_test.go index 9deca6edb..30fa58de3 100644 --- a/txpool/tx_object_test.go +++ b/txpool/tx_object_test.go @@ -34,15 +34,15 @@ func newTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx.BlockRef builder.Clause(c) } - tx, _ := builder.BlockRef(blockRef). + return tx.MustSignTx(builder.BlockRef(blockRef). Expiration(expiration). Nonce(rand.Uint64()). // nolint:gosec DependsOn(dependsOn). Features(features). Gas(gas). - BuildAndSign(from.PrivateKey) - - return tx + Build(), + from.PrivateKey, + ) } func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx.BlockRef, expiration uint32, dependsOn *thor.Bytes32, from genesis.DevAccount, delegator genesis.DevAccount) *tx.Transaction { @@ -54,15 +54,20 @@ func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx var features tx.Features features.SetDelegated(true) - tx, _ := builder.BlockRef(blockRef). + trx := builder.BlockRef(blockRef). Expiration(expiration). Nonce(rand.Uint64()). // nolint:gosec DependsOn(dependsOn). Features(features). Gas(gas). - BuildAndSignWithDelegator(from.PrivateKey, delegator.PrivateKey) + Build() + + trx, _ = tx.DelegatorSignTx( + tx.MustSignTx(trx, from.PrivateKey), + delegator.PrivateKey, + ) - return tx + return trx } func SetupTest() (genesis.DevAccount, *chain.Repository, *block.Block, *state.State) { From 113de60796c5cf47512ec3cc0a70ecf75c501227 Mon Sep 17 00:00:00 2001 From: otherview Date: Wed, 25 Sep 2024 14:12:39 +0100 Subject: [PATCH 3/8] tweaks + tests --- api/accounts/accounts_test.go | 2 +- api/blocks/blocks_test.go | 2 +- api/debug/debug_test.go | 4 +- api/subscriptions/block_reader_test.go | 2 +- api/subscriptions/pending_tx_test.go | 2 +- api/subscriptions/types_test.go | 4 +- api/transactions/transactions_test.go | 6 +-- cmd/thor/node/tx_stash_test.go | 2 +- cmd/thor/solo/solo.go | 2 +- consensus/consensus_test.go | 2 +- packer/packer_test.go | 2 +- runtime/resolved_tx_test.go | 2 +- tx/signer.go | 52 +++++++++++++-------- tx/signer_test.go | 63 ++++++++++++++++++++++++++ txpool/tx_object_test.go | 6 +-- 15 files changed, 116 insertions(+), 37 deletions(-) create mode 100644 tx/signer_test.go diff --git a/api/accounts/accounts_test.go b/api/accounts/accounts_test.go index 70590c318..6431d1934 100644 --- a/api/accounts/accounts_test.go +++ b/api/accounts/accounts_test.go @@ -294,7 +294,7 @@ func buildTxWithClauses(chaiTag byte, clauses ...*tx.Clause) *tx.Transaction { trx := builder.Build() - return tx.MustSignTx(trx, genesis.DevAccounts()[0].PrivateKey) + return tx.MustSign(trx, genesis.DevAccounts()[0].PrivateKey) } func packTx(repo *chain.Repository, stater *state.Stater, transaction *tx.Transaction, t *testing.T) { diff --git a/api/blocks/blocks_test.go b/api/blocks/blocks_test.go index dbaf8f0b6..4685e8080 100644 --- a/api/blocks/blocks_test.go +++ b/api/blocks/blocks_test.go @@ -170,7 +170,7 @@ func initBlockServer(t *testing.T) { repo, _ := chain.NewRepository(db, b) addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - trx := tx.MustSignTx( + trx := tx.MustSign( new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). diff --git a/api/debug/debug_test.go b/api/debug/debug_test.go index 3b7365d2c..8a8088e4d 100644 --- a/api/debug/debug_test.go +++ b/api/debug/debug_test.go @@ -531,7 +531,7 @@ func initDebugServer(t *testing.T) { Expiration(10). Gas(21000). Build() - noClausesTx = tx.MustSignTx(noClausesTx, genesis.DevAccounts()[0].PrivateKey) + noClausesTx = tx.MustSign(noClausesTx, genesis.DevAccounts()[0].PrivateKey) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) cla2 := tx.NewClause(&addr).WithValue(big.NewInt(10000)) @@ -545,7 +545,7 @@ func initDebugServer(t *testing.T) { Clause(cla2). BlockRef(tx.NewBlockRef(0)). Build() - transaction = tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) + transaction = tx.MustSign(transaction, genesis.DevAccounts()[0].PrivateKey) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/api/subscriptions/block_reader_test.go b/api/subscriptions/block_reader_test.go index 51bde8f97..f772ba389 100644 --- a/api/subscriptions/block_reader_test.go +++ b/api/subscriptions/block_reader_test.go @@ -85,7 +85,7 @@ func initChain(t *testing.T) (*chain.Repository, []*block.Block, *txpool.TxPool) Clause(cla). BlockRef(tx.NewBlockRef(0)). Build() - tr = tx.MustSignTx(tr, genesis.DevAccounts()[0].PrivateKey) + tr = tx.MustSign(tr, genesis.DevAccounts()[0].PrivateKey) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/api/subscriptions/pending_tx_test.go b/api/subscriptions/pending_tx_test.go index aa8314fa7..a4deb2c89 100644 --- a/api/subscriptions/pending_tx_test.go +++ b/api/subscriptions/pending_tx_test.go @@ -132,7 +132,7 @@ func createTx(repo *chain.Repository, addressNumber uint) *tx.Transaction { addr := thor.BytesToAddress([]byte("to")) cla := tx.NewClause(&addr).WithValue(big.NewInt(10000)) - return tx.MustSignTx( + return tx.MustSign( new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). diff --git a/api/subscriptions/types_test.go b/api/subscriptions/types_test.go index 8aee6b3b2..8cf1bbd07 100644 --- a/api/subscriptions/types_test.go +++ b/api/subscriptions/types_test.go @@ -103,7 +103,7 @@ func TestConvertTransfer(t *testing.T) { Nonce(1). BlockRef(tx.NewBlockRef(0)). Build() - transaction = tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) + transaction = tx.MustSign(transaction, genesis.DevAccounts()[0].PrivateKey) // New block blk := new(block.Builder). @@ -182,7 +182,7 @@ func TestConvertEvent(t *testing.T) { repo, _ := chain.NewRepository(db, b) // New tx - transaction := tx.MustSignTx( + transaction := tx.MustSign( new(tx.Builder). ChainTag(repo.ChainTag()). GasPriceCoef(1). diff --git a/api/transactions/transactions_test.go b/api/transactions/transactions_test.go index 9ec885762..697b617e9 100644 --- a/api/transactions/transactions_test.go +++ b/api/transactions/transactions_test.go @@ -111,7 +111,7 @@ func sendTx(t *testing.T) { var expiration = uint32(10) var gas = uint64(21000) - trx := tx.MustSignTx( + trx := tx.MustSign( new(tx.Builder). BlockRef(blockRef). ChainTag(chainTag). @@ -294,7 +294,7 @@ func initTransactionServer(t *testing.T) { Clause(cla). BlockRef(tx.NewBlockRef(0)). Build() - transaction = tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) + transaction = tx.MustSign(transaction, genesis.DevAccounts()[0].PrivateKey) mempoolTx = new(tx.Builder). ChainTag(repo.ChainTag()). @@ -302,7 +302,7 @@ func initTransactionServer(t *testing.T) { Gas(21000). Nonce(1). Build() - mempoolTx = tx.MustSignTx(mempoolTx, genesis.DevAccounts()[0].PrivateKey) + mempoolTx = tx.MustSign(mempoolTx, genesis.DevAccounts()[0].PrivateKey) packer := packer.New(repo, stater, genesis.DevAccounts()[0].Address, &genesis.DevAccounts()[0].Address, thor.NoFork) sum, _ := repo.GetBlockSummary(b.Header().ID()) diff --git a/cmd/thor/node/tx_stash_test.go b/cmd/thor/node/tx_stash_test.go index c236a9341..5fcd317b9 100644 --- a/cmd/thor/node/tx_stash_test.go +++ b/cmd/thor/node/tx_stash_test.go @@ -19,7 +19,7 @@ import ( ) func newTx() *tx.Transaction { - return tx.MustSignTx( + return tx.MustSign( new(tx.Builder). Nonce(rand.Uint64()).Build(), // nolint:gosec, genesis.DevAccounts()[0].PrivateKey, diff --git a/cmd/thor/solo/solo.go b/cmd/thor/solo/solo.go index 0d00cefc5..d177d3705 100644 --- a/cmd/thor/solo/solo.go +++ b/cmd/thor/solo/solo.go @@ -266,5 +266,5 @@ func (s *Solo) newTx(clauses []*tx.Clause, from genesis.DevAccount) (*tx.Transac DependsOn(nil). Gas(1_000_000). Build() - return tx.SignTx(trx, from.PrivateKey) + return tx.Sign(trx, from.PrivateKey) } diff --git a/consensus/consensus_test.go b/consensus/consensus_test.go index 73cead551..14bf7ec43 100644 --- a/consensus/consensus_test.go +++ b/consensus/consensus_test.go @@ -578,7 +578,7 @@ func TestValidateBlockBody(t *testing.T) { "TxOriginBlocked", func(t *testing.T) { thor.MockBlocklist([]string{genesis.DevAccounts()[9].Address.String()}) trx := txBuilder(tc.tag).Build() - trx = tx.MustSignTx(trx, genesis.DevAccounts()[9].PrivateKey) + trx = tx.MustSign(trx, genesis.DevAccounts()[9].PrivateKey) blk, err := tc.sign( tc.builder(tc.original.Header()).Transaction(trx), diff --git a/packer/packer_test.go b/packer/packer_test.go index c6829700d..da8078379 100644 --- a/packer/packer_test.go +++ b/packer/packer_test.go @@ -54,7 +54,7 @@ func (ti *txIterator) Next() *tx.Transaction { ChainTag(ti.chainTag). Clause(tx.NewClause(&builtin.Energy.Address).WithData(data)). Gas(300000).GasPriceCoef(0).Nonce(nonce).Expiration(math.MaxUint32).Build() - trx = tx.MustSignTx(trx, a0.PrivateKey) + trx = tx.MustSign(trx, a0.PrivateKey) nonce++ return trx diff --git a/runtime/resolved_tx_test.go b/runtime/resolved_tx_test.go index c0c521354..f0fbdb3cd 100644 --- a/runtime/resolved_tx_test.go +++ b/runtime/resolved_tx_test.go @@ -186,5 +186,5 @@ func txBuilder(tag byte) *tx.Builder { func txSign(builder *tx.Builder) *tx.Transaction { transaction := builder.Build() - return tx.MustSignTx(transaction, genesis.DevAccounts()[0].PrivateKey) + return tx.MustSign(transaction, genesis.DevAccounts()[0].PrivateKey) } diff --git a/tx/signer.go b/tx/signer.go index 913c0b21d..c32ce389b 100644 --- a/tx/signer.go +++ b/tx/signer.go @@ -1,3 +1,8 @@ +// Copyright (c) 2024 The VeChainThor developers + +// Distributed under the GNU Lesser General Public License v3.0 software license, see the accompanying +// file LICENSE or + package tx import ( @@ -5,38 +10,40 @@ import ( "fmt" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/secp256k1" + "github.com/pkg/errors" "github.com/vechain/thor/v2/thor" ) -// SignFunction is a type alias for a function that signs a hash using a private key. +// SignatureFunc is a type alias for a function that signs a hash using a private key. // It returns the signature and an error if any occurs. -type SignFunction func(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) +type SignatureFunc func(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) -// MustSignTx signs a transaction with the provided private key using the default signing function. +// MustSign signs a transaction with the provided private key using the default signing function. // It panics if the signing process fails. -func MustSignTx(tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { - return MustCustomSignTx(crypto.Sign, tx, pk) +func MustSign(tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { + return MustSignFunc(crypto.Sign, tx, pk) } -// MustCustomSignTx signs a transaction using a custom signing function and private key. +// MustSignFunc signs a transaction using a custom signing function and private key. // It panics if the signing process fails. -func MustCustomSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { - trx, err := CustomSignTx(sign, tx, pk) +func MustSignFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { + trx, err := SignFunc(sign, tx, pk) if err != nil { panic(err) } return trx } -// SignTx signs a transaction with the provided private key using the default signing function. +// Sign signs a transaction with the provided private key using the default signing function. // It returns the signed transaction or an error if the signing fails. -func SignTx(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { - return CustomSignTx(crypto.Sign, tx, pk) +func Sign(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + return SignFunc(crypto.Sign, tx, pk) } -// CustomSignTx signs a transaction using a custom signing function and private key. +// SignFunc signs a transaction using a custom signing function and private key. // It returns the signed transaction or an error if the signing fails. -func CustomSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { +func SignFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { // Generate the signature for the transaction's signing hash. sig, err := sign(tx.SigningHash().Bytes(), pk) if err != nil { @@ -47,15 +54,24 @@ func CustomSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) (*Tr return tx.WithSignature(sig), nil } -// DelegatorSignTx signs a transaction as a delegator with the provided private key using the default signing function. +// SignDelegator signs a transaction as a delegator with the provided private key using the default signing function. // It returns the signed transaction or an error if the signing fails. -func DelegatorSignTx(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { - return CustomDelegatorSignTx(crypto.Sign, tx, pk) +func SignDelegator(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + return SignDelegatorFunc(crypto.Sign, tx, pk) } -// CustomDelegatorSignTx signs a transaction as a delegator using a custom signing function and private key. +// SignDelegatorFunc signs a transaction as a delegator using a custom signing function and private key. // It returns the signed transaction or an error if the signing fails. -func CustomDelegatorSignTx(sign SignFunction, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { +func SignDelegatorFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { + // Must have the delegated feature enabled + if !tx.Features().IsDelegated() { + return nil, errors.New("transaction delegated feature is not enabled") + } + + // Must be signed by origin + if len(tx.Signature()) != 65 { + return nil, secp256k1.ErrInvalidSignatureLen + } // Extract the public key from the existing transaction signature. pub, err := crypto.SigToPub(tx.SigningHash().Bytes(), tx.Signature()) if err != nil { diff --git a/tx/signer_test.go b/tx/signer_test.go new file mode 100644 index 000000000..36b17eb60 --- /dev/null +++ b/tx/signer_test.go @@ -0,0 +1,63 @@ +// Copyright (c) 2024 The VeChainThor developers + +// Distributed under the GNU Lesser General Public License v3.0 software license, see the accompanying +// file LICENSE or + +package tx + +import ( + "crypto/rand" + "testing" + + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/secp256k1" + "github.com/stretchr/testify/assert" +) + +func TestSignTx(t *testing.T) { + // Generate a new private key for testing + pk, err := crypto.GenerateKey() + assert.NoError(t, err) + + tx := new(Builder).Build() + + // Sign the transaction + signedTx, err := Sign(tx, pk) + assert.NoError(t, err) + + // Verify the transaction was signed + assert.NotNil(t, signedTx) +} + +func TestDelegatorSignTx(t *testing.T) { + // Generate a new private key for testing + pk, err := crypto.GenerateKey() + assert.NoError(t, err) + + tx := new(Builder).Build() + + // Feature not enabled + signedTx, err := SignDelegator(tx, pk) + assert.ErrorContains(t, err, "transaction delegated feature is not enabled") + assert.Nil(t, signedTx) + + // enable the feature + var features Features + features.SetDelegated(true) + + // No valid Signature + tx = new(Builder).Features(features).Build() + signedTx, err = SignDelegator(tx, pk) + assert.ErrorIs(t, err, secp256k1.ErrInvalidSignatureLen) + assert.Nil(t, signedTx) + + // create a fake signature + fakeSig := [65]byte{} + rand.Read(fakeSig[:]) + tx = tx.WithSignature(fakeSig[:]) + + // Sign the transaction as a delegator + signedTx, err = SignDelegator(tx, pk) + assert.ErrorContains(t, err, "unable to extract public key from signature") + assert.Nil(t, signedTx) +} diff --git a/txpool/tx_object_test.go b/txpool/tx_object_test.go index 30fa58de3..5bb555dc2 100644 --- a/txpool/tx_object_test.go +++ b/txpool/tx_object_test.go @@ -34,7 +34,7 @@ func newTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx.BlockRef builder.Clause(c) } - return tx.MustSignTx(builder.BlockRef(blockRef). + return tx.MustSign(builder.BlockRef(blockRef). Expiration(expiration). Nonce(rand.Uint64()). // nolint:gosec DependsOn(dependsOn). @@ -62,8 +62,8 @@ func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx Gas(gas). Build() - trx, _ = tx.DelegatorSignTx( - tx.MustSignTx(trx, from.PrivateKey), + trx, _ = tx.SignDelegator( + tx.MustSign(trx, from.PrivateKey), delegator.PrivateKey, ) From 550a4f5a761b850bc460a408150fd89de4111a40 Mon Sep 17 00:00:00 2001 From: otherview Date: Thu, 26 Sep 2024 11:49:50 +0100 Subject: [PATCH 4/8] Updating method signatures --- tx/signer.go | 81 +++++++++++++++++++++------------------- tx/signer_test.go | 24 ++++++------ txpool/tx_object_test.go | 5 ++- 3 files changed, 57 insertions(+), 53 deletions(-) diff --git a/tx/signer.go b/tx/signer.go index c32ce389b..be7e1b147 100644 --- a/tx/signer.go +++ b/tx/signer.go @@ -19,77 +19,82 @@ import ( // It returns the signature and an error if any occurs. type SignatureFunc func(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) -// MustSign signs a transaction with the provided private key using the default signing function. -// It panics if the signing process fails. +// MustSign signs a transaction using the provided private key and the default signing function. +// It panics if the signing process fails, returning a signed transaction upon success. func MustSign(tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { - return MustSignFunc(crypto.Sign, tx, pk) -} - -// MustSignFunc signs a transaction using a custom signing function and private key. -// It panics if the signing process fails. -func MustSignFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { - trx, err := SignFunc(sign, tx, pk) + trx, err := signFunc(crypto.Sign, tx, pk) if err != nil { panic(err) } return trx } -// Sign signs a transaction with the provided private key using the default signing function. -// It returns the signed transaction or an error if the signing fails. +// Sign signs a transaction using the provided private key and the default signing function. +// It returns the signed transaction or an error if the signing process fails. func Sign(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { - return SignFunc(crypto.Sign, tx, pk) + return signFunc(crypto.Sign, tx, pk) } -// SignFunc signs a transaction using a custom signing function and private key. -// It returns the signed transaction or an error if the signing fails. -func SignFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { +// signFunc signs a transaction using a custom signing function and the provided private key. +// It returns the signed transaction or an error if the signing process fails. +func signFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { // Generate the signature for the transaction's signing hash. sig, err := sign(tx.SigningHash().Bytes(), pk) if err != nil { return nil, fmt.Errorf("unable to sign transaction: %w", err) } - // Attach the signature to the transaction. + // Attach the signature to the transaction and return the signed transaction. return tx.WithSignature(sig), nil } -// SignDelegator signs a transaction as a delegator with the provided private key using the default signing function. -// It returns the signed transaction or an error if the signing fails. -func SignDelegator(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { - return SignDelegatorFunc(crypto.Sign, tx, pk) +// MustSignDelegator signs a transaction as a delegator using the provided private keys and the default signing function. +// It panics if the signing process fails, returning a signed transaction upon success. +func MustSignDelegator(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) *Transaction { + trx, err := signDelegatorFunc(crypto.Sign, tx, originPK, delegatorPK) + if err != nil { + panic(err) + } + return trx +} + +// SignDelegator signs a transaction as a delegator using the provided private keys and the default signing function. +// It returns the signed transaction or an error if the signing process fails. +func SignDelegator(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { + return signDelegatorFunc(crypto.Sign, tx, originPK, delegatorPK) } -// SignDelegatorFunc signs a transaction as a delegator using a custom signing function and private key. -// It returns the signed transaction or an error if the signing fails. -func SignDelegatorFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { - // Must have the delegated feature enabled - if !tx.Features().IsDelegated() { +// signDelegatorFunc signs a transaction as a delegator using a custom signing function and the provided private keys. +// It returns the signed transaction or an error if the signing process fails. +func signDelegatorFunc(sign SignatureFunc, unsignedTx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { + // Ensure the transaction has the delegated feature enabled. + if !unsignedTx.Features().IsDelegated() { return nil, errors.New("transaction delegated feature is not enabled") } - // Must be signed by origin - if len(tx.Signature()) != 65 { + // Ensure the transaction has not already been signed by the origin. + if len(unsignedTx.Signature()) != 0 { return nil, secp256k1.ErrInvalidSignatureLen } - // Extract the public key from the existing transaction signature. - pub, err := crypto.SigToPub(tx.SigningHash().Bytes(), tx.Signature()) + + // Sign the transaction using the origin's private key. + signedTx, err := Sign(unsignedTx, originPK) if err != nil { - return nil, fmt.Errorf("unable to extract public key from signature: %w", err) + return nil, err } - // Convert the public key to the corresponding address. - origin := thor.Address(crypto.PubkeyToAddress(*pub)) + // Convert the origin's public key to its corresponding address. + origin := thor.Address(crypto.PubkeyToAddress(originPK.PublicKey)) - // Generate the delegator's signature using the delegator signing hash. - dSig, err := sign(tx.DelegatorSigningHash(origin).Bytes(), pk) + // Generate the delegator's signature using the transaction's delegator signing hash. + dSig, err := sign(signedTx.DelegatorSigningHash(origin).Bytes(), delegatorPK) if err != nil { return nil, fmt.Errorf("unable to delegator sign transaction: %w", err) } - // Append the delegator's signature to the existing transaction signature. - sig := append(tx.Signature(), dSig...) + // Append the delegator's signature to the origin's signature. + sig := append(signedTx.Signature(), dSig...) - // Attach the combined signature to the transaction. - return tx.WithSignature(sig), nil + // Attach the combined signature to the transaction and return the signed transaction. + return signedTx.WithSignature(sig), nil } diff --git a/tx/signer_test.go b/tx/signer_test.go index 36b17eb60..9b17dadc1 100644 --- a/tx/signer_test.go +++ b/tx/signer_test.go @@ -6,7 +6,6 @@ package tx import ( - "crypto/rand" "testing" "github.com/ethereum/go-ethereum/crypto" @@ -31,13 +30,16 @@ func TestSignTx(t *testing.T) { func TestDelegatorSignTx(t *testing.T) { // Generate a new private key for testing - pk, err := crypto.GenerateKey() + delegatorPK, err := crypto.GenerateKey() + assert.NoError(t, err) + + originPK, err := crypto.GenerateKey() assert.NoError(t, err) tx := new(Builder).Build() // Feature not enabled - signedTx, err := SignDelegator(tx, pk) + signedTx, err := SignDelegator(tx, originPK, delegatorPK) assert.ErrorContains(t, err, "transaction delegated feature is not enabled") assert.Nil(t, signedTx) @@ -45,19 +47,15 @@ func TestDelegatorSignTx(t *testing.T) { var features Features features.SetDelegated(true) - // No valid Signature + // tx is already signed tx = new(Builder).Features(features).Build() - signedTx, err = SignDelegator(tx, pk) + signedTx = MustSign(tx, originPK) + signedTx, err = SignDelegator(signedTx, originPK, delegatorPK) assert.ErrorIs(t, err, secp256k1.ErrInvalidSignatureLen) assert.Nil(t, signedTx) - // create a fake signature - fakeSig := [65]byte{} - rand.Read(fakeSig[:]) - tx = tx.WithSignature(fakeSig[:]) - // Sign the transaction as a delegator - signedTx, err = SignDelegator(tx, pk) - assert.ErrorContains(t, err, "unable to extract public key from signature") - assert.Nil(t, signedTx) + signedTx, err = SignDelegator(tx, originPK, delegatorPK) + assert.NoError(t, err) + assert.NotNil(t, signedTx) } diff --git a/txpool/tx_object_test.go b/txpool/tx_object_test.go index 5bb555dc2..5d111f6b2 100644 --- a/txpool/tx_object_test.go +++ b/txpool/tx_object_test.go @@ -62,8 +62,9 @@ func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx Gas(gas). Build() - trx, _ = tx.SignDelegator( - tx.MustSign(trx, from.PrivateKey), + trx = tx.MustSignDelegator( + trx, + from.PrivateKey, delegator.PrivateKey, ) From 6b062f6ad74c3202a9ed7ab2eee6723806bda2a9 Mon Sep 17 00:00:00 2001 From: otherview Date: Thu, 26 Sep 2024 12:02:06 +0100 Subject: [PATCH 5/8] pr comments --- tx/signer.go | 32 ++++++++------------------------ tx/signer_test.go | 10 +++++----- txpool/tx_object_test.go | 2 +- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/tx/signer.go b/tx/signer.go index be7e1b147..65bd7b078 100644 --- a/tx/signer.go +++ b/tx/signer.go @@ -15,14 +15,10 @@ import ( "github.com/vechain/thor/v2/thor" ) -// SignatureFunc is a type alias for a function that signs a hash using a private key. -// It returns the signature and an error if any occurs. -type SignatureFunc func(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) - // MustSign signs a transaction using the provided private key and the default signing function. // It panics if the signing process fails, returning a signed transaction upon success. func MustSign(tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { - trx, err := signFunc(crypto.Sign, tx, pk) + trx, err := Sign(tx, pk) if err != nil { panic(err) } @@ -32,14 +28,8 @@ func MustSign(tx *Transaction, pk *ecdsa.PrivateKey) *Transaction { // Sign signs a transaction using the provided private key and the default signing function. // It returns the signed transaction or an error if the signing process fails. func Sign(tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { - return signFunc(crypto.Sign, tx, pk) -} - -// signFunc signs a transaction using a custom signing function and the provided private key. -// It returns the signed transaction or an error if the signing process fails. -func signFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Transaction, error) { // Generate the signature for the transaction's signing hash. - sig, err := sign(tx.SigningHash().Bytes(), pk) + sig, err := crypto.Sign(tx.SigningHash().Bytes(), pk) if err != nil { return nil, fmt.Errorf("unable to sign transaction: %w", err) } @@ -48,25 +38,19 @@ func signFunc(sign SignatureFunc, tx *Transaction, pk *ecdsa.PrivateKey) (*Trans return tx.WithSignature(sig), nil } -// MustSignDelegator signs a transaction as a delegator using the provided private keys and the default signing function. +// MustSignDelegated signs a transaction as a delegator using the provided private keys and the default signing function. // It panics if the signing process fails, returning a signed transaction upon success. -func MustSignDelegator(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) *Transaction { - trx, err := signDelegatorFunc(crypto.Sign, tx, originPK, delegatorPK) +func MustSignDelegated(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) *Transaction { + trx, err := SignDelegated(tx, originPK, delegatorPK) if err != nil { panic(err) } return trx } -// SignDelegator signs a transaction as a delegator using the provided private keys and the default signing function. -// It returns the signed transaction or an error if the signing process fails. -func SignDelegator(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { - return signDelegatorFunc(crypto.Sign, tx, originPK, delegatorPK) -} - -// signDelegatorFunc signs a transaction as a delegator using a custom signing function and the provided private keys. +// SignDelegated signs a transaction as a delegator using the provided private keys and the default signing function. // It returns the signed transaction or an error if the signing process fails. -func signDelegatorFunc(sign SignatureFunc, unsignedTx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { +func SignDelegated(unsignedTx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { // Ensure the transaction has the delegated feature enabled. if !unsignedTx.Features().IsDelegated() { return nil, errors.New("transaction delegated feature is not enabled") @@ -87,7 +71,7 @@ func signDelegatorFunc(sign SignatureFunc, unsignedTx *Transaction, originPK *ec origin := thor.Address(crypto.PubkeyToAddress(originPK.PublicKey)) // Generate the delegator's signature using the transaction's delegator signing hash. - dSig, err := sign(signedTx.DelegatorSigningHash(origin).Bytes(), delegatorPK) + dSig, err := crypto.Sign(signedTx.DelegatorSigningHash(origin).Bytes(), delegatorPK) if err != nil { return nil, fmt.Errorf("unable to delegator sign transaction: %w", err) } diff --git a/tx/signer_test.go b/tx/signer_test.go index 9b17dadc1..f6052bb76 100644 --- a/tx/signer_test.go +++ b/tx/signer_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestSignTx(t *testing.T) { +func TestSign(t *testing.T) { // Generate a new private key for testing pk, err := crypto.GenerateKey() assert.NoError(t, err) @@ -28,7 +28,7 @@ func TestSignTx(t *testing.T) { assert.NotNil(t, signedTx) } -func TestDelegatorSignTx(t *testing.T) { +func TestSignDelegated(t *testing.T) { // Generate a new private key for testing delegatorPK, err := crypto.GenerateKey() assert.NoError(t, err) @@ -39,7 +39,7 @@ func TestDelegatorSignTx(t *testing.T) { tx := new(Builder).Build() // Feature not enabled - signedTx, err := SignDelegator(tx, originPK, delegatorPK) + signedTx, err := SignDelegated(tx, originPK, delegatorPK) assert.ErrorContains(t, err, "transaction delegated feature is not enabled") assert.Nil(t, signedTx) @@ -50,12 +50,12 @@ func TestDelegatorSignTx(t *testing.T) { // tx is already signed tx = new(Builder).Features(features).Build() signedTx = MustSign(tx, originPK) - signedTx, err = SignDelegator(signedTx, originPK, delegatorPK) + signedTx, err = SignDelegated(signedTx, originPK, delegatorPK) assert.ErrorIs(t, err, secp256k1.ErrInvalidSignatureLen) assert.Nil(t, signedTx) // Sign the transaction as a delegator - signedTx, err = SignDelegator(tx, originPK, delegatorPK) + signedTx, err = SignDelegated(tx, originPK, delegatorPK) assert.NoError(t, err) assert.NotNil(t, signedTx) } diff --git a/txpool/tx_object_test.go b/txpool/tx_object_test.go index 5d111f6b2..5eb90d78a 100644 --- a/txpool/tx_object_test.go +++ b/txpool/tx_object_test.go @@ -62,7 +62,7 @@ func newDelegatedTx(chainTag byte, clauses []*tx.Clause, gas uint64, blockRef tx Gas(gas). Build() - trx = tx.MustSignDelegator( + trx = tx.MustSignDelegated( trx, from.PrivateKey, delegator.PrivateKey, From 2367e97e67ac0dc06808355e73299e815bbb743b Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Wed, 9 Oct 2024 09:34:04 +0100 Subject: [PATCH 6/8] Update tx/signer.go Co-authored-by: libotony --- tx/signer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tx/signer.go b/tx/signer.go index 65bd7b078..3323234e0 100644 --- a/tx/signer.go +++ b/tx/signer.go @@ -48,7 +48,7 @@ func MustSignDelegated(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK return trx } -// SignDelegated signs a transaction as a delegator using the provided private keys and the default signing function. +// SignDelegated signs a transaction with both origin and delegator's private key and the default signing function. // It returns the signed transaction or an error if the signing process fails. func SignDelegated(unsignedTx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { // Ensure the transaction has the delegated feature enabled. From 5c68ddff5e1efba88d619433badeeeecfe8ed86d Mon Sep 17 00:00:00 2001 From: otherview Date: Wed, 9 Oct 2024 09:55:22 +0100 Subject: [PATCH 7/8] pr comments --- tx/signer.go | 21 ++++++++------------- tx/signer_test.go | 14 +++++++------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/tx/signer.go b/tx/signer.go index 65bd7b078..5de79a1e8 100644 --- a/tx/signer.go +++ b/tx/signer.go @@ -10,7 +10,6 @@ import ( "fmt" "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/crypto/secp256k1" "github.com/pkg/errors" "github.com/vechain/thor/v2/thor" ) @@ -50,35 +49,31 @@ func MustSignDelegated(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK // SignDelegated signs a transaction as a delegator using the provided private keys and the default signing function. // It returns the signed transaction or an error if the signing process fails. -func SignDelegated(unsignedTx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { +func SignDelegated(tx *Transaction, originPK *ecdsa.PrivateKey, delegatorPK *ecdsa.PrivateKey) (*Transaction, error) { // Ensure the transaction has the delegated feature enabled. - if !unsignedTx.Features().IsDelegated() { + if !tx.Features().IsDelegated() { return nil, errors.New("transaction delegated feature is not enabled") } - // Ensure the transaction has not already been signed by the origin. - if len(unsignedTx.Signature()) != 0 { - return nil, secp256k1.ErrInvalidSignatureLen - } - // Sign the transaction using the origin's private key. - signedTx, err := Sign(unsignedTx, originPK) + // Generate the signature for the transaction's signing hash. + originSig, err := crypto.Sign(tx.SigningHash().Bytes(), originPK) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to sign transaction: %w", err) } // Convert the origin's public key to its corresponding address. origin := thor.Address(crypto.PubkeyToAddress(originPK.PublicKey)) // Generate the delegator's signature using the transaction's delegator signing hash. - dSig, err := crypto.Sign(signedTx.DelegatorSigningHash(origin).Bytes(), delegatorPK) + dSig, err := crypto.Sign(tx.DelegatorSigningHash(origin).Bytes(), delegatorPK) if err != nil { return nil, fmt.Errorf("unable to delegator sign transaction: %w", err) } // Append the delegator's signature to the origin's signature. - sig := append(signedTx.Signature(), dSig...) + sig := append(originSig, dSig...) // Attach the combined signature to the transaction and return the signed transaction. - return signedTx.WithSignature(sig), nil + return tx.WithSignature(sig), nil } diff --git a/tx/signer_test.go b/tx/signer_test.go index f6052bb76..19ba77b28 100644 --- a/tx/signer_test.go +++ b/tx/signer_test.go @@ -9,8 +9,9 @@ import ( "testing" "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/crypto/secp256k1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vechain/thor/v2/thor" ) func TestSign(t *testing.T) { @@ -26,6 +27,11 @@ func TestSign(t *testing.T) { // Verify the transaction was signed assert.NotNil(t, signedTx) + + // Verify address from Origin + addr, err := signedTx.Origin() + require.NoError(t, err) + assert.Equal(t, thor.Address(crypto.PubkeyToAddress(pk.PublicKey)), addr) } func TestSignDelegated(t *testing.T) { @@ -46,13 +52,7 @@ func TestSignDelegated(t *testing.T) { // enable the feature var features Features features.SetDelegated(true) - - // tx is already signed tx = new(Builder).Features(features).Build() - signedTx = MustSign(tx, originPK) - signedTx, err = SignDelegated(signedTx, originPK, delegatorPK) - assert.ErrorIs(t, err, secp256k1.ErrInvalidSignatureLen) - assert.Nil(t, signedTx) // Sign the transaction as a delegator signedTx, err = SignDelegated(tx, originPK, delegatorPK) From 5c159457569391f0290e23a99e7e38873e5c64f6 Mon Sep 17 00:00:00 2001 From: tony Date: Wed, 9 Oct 2024 17:39:04 +0800 Subject: [PATCH 8/8] add more tests --- tx/signer_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tx/signer_test.go b/tx/signer_test.go index 19ba77b28..5a11fa62c 100644 --- a/tx/signer_test.go +++ b/tx/signer_test.go @@ -32,6 +32,11 @@ func TestSign(t *testing.T) { addr, err := signedTx.Origin() require.NoError(t, err) assert.Equal(t, thor.Address(crypto.PubkeyToAddress(pk.PublicKey)), addr) + + // Verify the delegator + delegator, err := signedTx.Delegator() + require.NoError(t, err) + assert.Nil(t, delegator) } func TestSignDelegated(t *testing.T) { @@ -58,4 +63,14 @@ func TestSignDelegated(t *testing.T) { signedTx, err = SignDelegated(tx, originPK, delegatorPK) assert.NoError(t, err) assert.NotNil(t, signedTx) + + // Verify address from Origin + origin, err := signedTx.Origin() + require.NoError(t, err) + assert.Equal(t, thor.Address(crypto.PubkeyToAddress(originPK.PublicKey)), origin) + + // Verify the delegator + delegator, err := signedTx.Delegator() + require.NoError(t, err) + assert.Equal(t, thor.Address(crypto.PubkeyToAddress(delegatorPK.PublicKey)), *delegator) }