From 3832b1312d7de4db90aed28ac861331cf1bc4c31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Mon, 29 Apr 2024 14:01:25 +0200 Subject: [PATCH] fix: remove empty timestamp from the tx signature payload (#1939) ## Description This PR closes #810 I've dropped the unused `Timestamp` field from the `SignDoc`, which forced all clients that generate tx signatures outside of the `gno` repo to initialize it to an empty UTC timestamp value (ex. Adena, `tm2-js-client`...) **BREAKING CHANGE**: all clients that generate transaction signatures without using the provided Go methods in this repository need to update their source code to remove the `Timestamp` field. (@dongwon8247 @jefft0) I've also provided error handling for generating these signature bytes, which weren't present before.
Contributors' checklist... - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
--- gno.land/pkg/gnoclient/signer.go | 5 +- tm2/pkg/crypto/keys/client/sign.go | 16 +++-- tm2/pkg/crypto/keys/client/sign_test.go | 10 ++- tm2/pkg/sdk/auth/ante.go | 21 ++++-- tm2/pkg/sdk/auth/ante_test.go | 87 ++++++++++++++----------- tm2/pkg/sdk/testutils/testutils.go | 49 ++++++++++++-- tm2/pkg/std/doc.go | 60 ++++++++--------- tm2/pkg/std/doc_test.go | 50 ++++++++++++++ tm2/pkg/std/tx.go | 11 +++- tm2/pkg/std/utils.go | 18 ++--- 10 files changed, 228 insertions(+), 99 deletions(-) create mode 100644 tm2/pkg/std/doc_test.go diff --git a/gno.land/pkg/gnoclient/signer.go b/gno.land/pkg/gnoclient/signer.go index 4d40680037e..f8e1e6b8522 100644 --- a/gno.land/pkg/gnoclient/signer.go +++ b/gno.land/pkg/gnoclient/signer.go @@ -96,7 +96,10 @@ func (s SignerFromKeybase) Sign(cfg SignCfg) (*std.Tx, error) { } // Derive sign doc bytes. - signbz := tx.GetSignBytes(chainID, accountNumber, sequenceNumber) + signbz, err := tx.GetSignBytes(chainID, accountNumber, sequenceNumber) + if err != nil { + return nil, fmt.Errorf("unable to get tx signature payload, %w", err) + } sig, pub, err := s.Keybase.Sign(account, password, signbz) if err != nil { diff --git a/tm2/pkg/crypto/keys/client/sign.go b/tm2/pkg/crypto/keys/client/sign.go index 160b7a3447f..87833165063 100644 --- a/tm2/pkg/crypto/keys/client/sign.go +++ b/tm2/pkg/crypto/keys/client/sign.go @@ -185,15 +185,21 @@ func signTx( signOpts signOpts, keyOpts keyOpts, ) error { + signBytes, err := tx.GetSignBytes( + signOpts.chainID, + signOpts.accountNumber, + signOpts.accountSequence, + ) + if err != nil { + return fmt.Errorf("unable to get signature bytes, %w", err) + } + // Sign the transaction data sig, pub, err := kb.Sign( keyOpts.keyName, keyOpts.decryptPass, - tx.GetSignBytes( - signOpts.chainID, - signOpts.accountNumber, - signOpts.accountSequence, - )) + signBytes, + ) if err != nil { return fmt.Errorf("unable to sign transaction bytes, %w", err) } diff --git a/tm2/pkg/crypto/keys/client/sign_test.go b/tm2/pkg/crypto/keys/client/sign_test.go index d1a2dafaa61..eb30dc17162 100644 --- a/tm2/pkg/crypto/keys/client/sign_test.go +++ b/tm2/pkg/crypto/keys/client/sign_test.go @@ -444,7 +444,10 @@ func TestSign_SignTx(t *testing.T) { require.NoError(t, err) // Generate the signature - signature, pubKey, err := kb.Sign(anotherKey, encryptPassword, tx.GetSignBytes("id", 1, 0)) + signBytes, err := tx.GetSignBytes("id", 1, 0) + require.NoError(t, err) + + signature, pubKey, err := kb.Sign(anotherKey, encryptPassword, signBytes) require.NoError(t, err) tx.Signatures = []std.Signature{ @@ -554,7 +557,10 @@ func TestSign_SignTx(t *testing.T) { require.NoError(t, err) // Generate the signature - signature, pubKey, err := kb.Sign(keyName, encryptPassword, tx.GetSignBytes("id", 0, 0)) + signBytes, err := tx.GetSignBytes("id", 0, 0) + require.NoError(t, err) + + signature, pubKey, err := kb.Sign(keyName, encryptPassword, signBytes) require.NoError(t, err) tx.Signatures = []std.Signature{ diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index a0ab1cb1dd0..5066a7b1fde 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -153,7 +153,11 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature // No signatures are needed for genesis. } else { // Check signature - signBytes := GetSignBytes(newCtx.ChainID(), tx, sacc, isGenesis) + signBytes, err := GetSignBytes(newCtx.ChainID(), tx, sacc, isGenesis) + if err != nil { + return newCtx, res, true + } + signerAccs[i], res = processSig(newCtx, sacc, stdSigs[i], signBytes, simulate, params, sigGasConsumer) if !res.IsOK() { return newCtx, res, true @@ -430,15 +434,22 @@ func SetGasMeter(simulate bool, ctx sdk.Context, gasLimit int64) sdk.Context { // GetSignBytes returns a slice of bytes to sign over for a given transaction // and an account. -func GetSignBytes(chainID string, tx std.Tx, acc std.Account, genesis bool) []byte { +func GetSignBytes(chainID string, tx std.Tx, acc std.Account, genesis bool) ([]byte, error) { var accNum uint64 if !genesis { accNum = acc.GetAccountNumber() } - signbz := std.SignBytes( - chainID, accNum, acc.GetSequence(), tx.Fee, tx.Msgs, tx.Memo, + + return std.GetSignaturePayload( + std.SignDoc{ + ChainID: chainID, + AccountNumber: accNum, + Sequence: acc.GetSequence(), + Fee: tx.Fee, + Msgs: tx.Msgs, + Memo: tx.Memo, + }, ) - return signbz } func abciResult(err error) sdk.Result { diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index 9c15cca18a2..be4167a6238 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -81,7 +81,7 @@ func TestAnteHandlerSigErrors(t *testing.T) { // test no signatures privs, accNums, seqs := []crypto.PrivKey{}, []uint64{}, []uint64{} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accNums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accNums, seqs, fee) // tx.GetSigners returns addresses in correct order: addr1, addr2, addr3 expectedSigners := []crypto.Address{addr1, addr2, addr3} @@ -92,12 +92,12 @@ func TestAnteHandlerSigErrors(t *testing.T) { // test num sigs dont match GetSigners privs, accNums, seqs = []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accNums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accNums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // test an unrecognized account privs, accNums, seqs = []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accNums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accNums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnknownAddressError{}) // save the first account, but second is still unrecognized @@ -139,17 +139,17 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { // test good tx from one signer privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // new tx from wrong account number seqs = []uint64{1} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // from correct account number seqs = []uint64{1} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, []uint64{0}, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, []uint64{0}, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // new tx with another signer and incorrect account numbers @@ -157,12 +157,12 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { msg2 := tu.NewTestMsg(addr2, addr1) msgs = []std.Msg{msg1, msg2} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{1, 0}, []uint64{2, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // correct account numbers privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{2, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } @@ -200,17 +200,17 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { // test good tx from one signer privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // new tx from wrong account number seqs = []uint64{1} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // from correct account number seqs = []uint64{1} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, []uint64{0}, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, []uint64{0}, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // new tx with another signer and incorrect account numbers @@ -218,12 +218,12 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { msg2 := tu.NewTestMsg(addr2, addr1) msgs = []std.Msg{msg1, msg2} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{1, 0}, []uint64{2, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // correct account numbers privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{0, 0}, []uint64{2, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } @@ -264,7 +264,7 @@ func TestAnteHandlerSequences(t *testing.T) { // test good tx from one signer privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // test sending it again fails (replay protection) @@ -272,7 +272,7 @@ func TestAnteHandlerSequences(t *testing.T) { // fix sequence, should pass seqs = []uint64{1} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // new tx with another signer and correct sequences @@ -281,7 +281,7 @@ func TestAnteHandlerSequences(t *testing.T) { msgs = []std.Msg{msg1, msg2} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{2, 0, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) // replay fails @@ -291,18 +291,18 @@ func TestAnteHandlerSequences(t *testing.T) { msg = tu.NewTestMsg(addr2) msgs = []std.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv2}, []uint64{1}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // fix the sequence and it passes - tx = tu.NewTestTx(ctx.ChainID(), msgs, []crypto.PrivKey{priv2}, []uint64{1}, []uint64{1}, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, []crypto.PrivKey{priv2}, []uint64{1}, []uint64{1}, fee) checkValidTx(t, anteHandler, ctx, tx, false) // another tx from both of them that passes msg = tu.NewTestMsg(addr1, addr2) msgs = []std.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{3, 2} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } @@ -330,7 +330,7 @@ func TestAnteHandlerFees(t *testing.T) { msgs := []std.Msg{msg} // signer does not have enough funds to pay the fee - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.InsufficientFundsError{}) acc1.SetCoins(std.NewCoins(std.NewCoin("atom", 149))) @@ -373,22 +373,22 @@ func TestAnteHandlerMemoGas(t *testing.T) { fee := std.NewFee(0, std.NewCoin("atom", 0)) // tx does not have enough gas - tx = tu.NewTestTx(ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.OutOfGasError{}) // tx with memo doesn't have enough gas fee = std.NewFee(801, std.NewCoin("atom", 0)) - tx = tu.NewTestTxWithMemo(ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd") + tx = tu.NewTestTxWithMemo(t, ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd") checkInvalidTx(t, anteHandler, ctx, tx, false, std.OutOfGasError{}) // memo too large fee = std.NewFee(9000, std.NewCoin("atom", 0)) - tx = tu.NewTestTxWithMemo(ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("01234567890", 99000)) + tx = tu.NewTestTxWithMemo(t, ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("01234567890", 99000)) checkInvalidTx(t, anteHandler, ctx, tx, false, std.MemoTooLargeError{}) // tx with memo has enough gas fee = std.NewFee(9000, std.NewCoin("atom", 0)) - tx = tu.NewTestTxWithMemo(ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("0123456789", 10)) + tx = tu.NewTestTxWithMemo(t, ctx.ChainID(), []std.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("0123456789", 10)) checkValidTx(t, anteHandler, ctx, tx, false) } @@ -429,18 +429,18 @@ func TestAnteHandlerMultiSigner(t *testing.T) { // signers in order privs, accnums, seqs := []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0} - tx = tu.NewTestTxWithMemo(ctx.ChainID(), msgs, privs, accnums, seqs, fee, "Check signers are in expected order and different account numbers works") + tx = tu.NewTestTxWithMemo(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee, "Check signers are in expected order and different account numbers works") checkValidTx(t, anteHandler, ctx, tx, false) // change sequence numbers - tx = tu.NewTestTx(ctx.ChainID(), []std.Msg{msg1}, []crypto.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{1, 1}, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), []std.Msg{msg1}, []crypto.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{1, 1}, fee) checkValidTx(t, anteHandler, ctx, tx, false) - tx = tu.NewTestTx(ctx.ChainID(), []std.Msg{msg2}, []crypto.PrivKey{priv3, priv1}, []uint64{2, 0}, []uint64{1, 2}, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), []std.Msg{msg2}, []crypto.PrivKey{priv3, priv1}, []uint64{2, 0}, []uint64{1, 2}, fee) checkValidTx(t, anteHandler, ctx, tx, false) // expected seqs = [3, 2, 2] - tx = tu.NewTestTxWithMemo(ctx.ChainID(), msgs, privs, accnums, []uint64{3, 2, 2}, fee, "Check signers are in expected order and different account numbers and sequence numbers works") + tx = tu.NewTestTxWithMemo(t, ctx.ChainID(), msgs, privs, accnums, []uint64{3, 2, 2}, fee, "Check signers are in expected order and different account numbers and sequence numbers works") checkValidTx(t, anteHandler, ctx, tx, false) } @@ -477,7 +477,7 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // test good tx and signBytes privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) chainID := ctx.ChainID() @@ -502,9 +502,18 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { privs, seqs = []crypto.PrivKey{priv1}, []uint64{1} for _, cs := range cases { + signPayload, err := std.GetSignaturePayload(std.SignDoc{ + ChainID: cs.chainID, + AccountNumber: cs.accnum, + Sequence: cs.seq, + Fee: cs.fee, + Msgs: cs.msgs, + }) + require.NoError(t, err) + tx := tu.NewTestTxWithSignBytes( - msgs, privs, accnums, seqs, fee, - std.SignBytes(cs.chainID, cs.accnum, cs.seq, cs.fee, cs.msgs, ""), + msgs, privs, fee, + signPayload, "", ) checkInvalidTx(t, anteHandler, ctx, tx, false, cs.err) @@ -512,14 +521,14 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // test wrong signer if public key exist privs, accnums, seqs = []crypto.PrivKey{priv2}, []uint64{0}, []uint64{1} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.UnauthorizedError{}) // test wrong signer if public doesn't exist msg = tu.NewTestMsg(addr2) msgs = []std.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv1}, []uint64{1}, []uint64{0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.InvalidPubKeyError{}) } @@ -552,7 +561,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { msgs := []std.Msg{msg} privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} fee := tu.NewTestFee() - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) acc1 = env.acck.GetAccount(ctx, addr1) @@ -561,7 +570,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { // test public key not found msg = tu.NewTestMsg(addr2) msgs = []std.Msg{msg} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) sigs := tx.GetSignatures() sigs[0].PubKey = nil checkInvalidTx(t, anteHandler, ctx, tx, false, std.InvalidPubKeyError{}) @@ -570,7 +579,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { require.Nil(t, acc2.GetPubKey()) // test invalid signature and public key - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, []uint64{1}, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.InvalidPubKeyError{}) acc2 = env.acck.GetAccount(ctx, addr2) @@ -772,7 +781,7 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { // test rejection logic privs, accnums, seqs := []crypto.PrivKey{priv1, priv2, priv3, priv4, priv5, priv6, priv7, priv8}, []uint64{0, 0, 0, 0, 0, 0, 0, 0}, []uint64{0, 0, 0, 0, 0, 0, 0, 0} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.TooManySignaturesError{}) } @@ -839,7 +848,7 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} fee := tu.NewTestFee() msgs := []std.Msg{msg} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, std.InvalidPubKeyError{}) // verify that an ed25519 account gets accepted @@ -854,6 +863,6 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { privs, accnums, seqs = []crypto.PrivKey{priv2}, []uint64{1}, []uint64{0} fee = tu.NewTestFee() msgs = []std.Msg{msg} - tx = tu.NewTestTx(ctx.ChainID(), msgs, privs, accnums, seqs, fee) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } diff --git a/tm2/pkg/sdk/testutils/testutils.go b/tm2/pkg/sdk/testutils/testutils.go index ffbef18863c..f1933f36ec8 100644 --- a/tm2/pkg/sdk/testutils/testutils.go +++ b/tm2/pkg/sdk/testutils/testutils.go @@ -1,10 +1,13 @@ package testutils import ( + "testing" + "github.com/gnolang/gno/tm2/pkg/amino" "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1" "github.com/gnolang/gno/tm2/pkg/std" + "github.com/stretchr/testify/require" ) // msg type for testing @@ -53,10 +56,27 @@ func KeyTestPubAddr() (crypto.PrivKey, crypto.PubKey, crypto.Address) { return key, pub, addr } -func NewTestTx(chainID string, msgs []std.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee std.Fee) std.Tx { +func NewTestTx( + t *testing.T, + chainID string, + msgs []std.Msg, + privs []crypto.PrivKey, + accNums []uint64, + seqs []uint64, + fee std.Fee, +) std.Tx { + t.Helper() + sigs := make([]std.Signature, len(privs)) for i, priv := range privs { - signBytes := std.SignBytes(chainID, accNums[i], seqs[i], fee, msgs, "") + signBytes, err := std.GetSignaturePayload(std.SignDoc{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: seqs[i], + Fee: fee, + Msgs: msgs, + }) + require.NoError(t, err) sig, err := priv.Sign(signBytes) if err != nil { @@ -70,10 +90,29 @@ func NewTestTx(chainID string, msgs []std.Msg, privs []crypto.PrivKey, accNums [ return tx } -func NewTestTxWithMemo(chainID string, msgs []std.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee std.Fee, memo string) std.Tx { +func NewTestTxWithMemo( + t *testing.T, + chainID string, + msgs []std.Msg, + privs []crypto.PrivKey, + accNums []uint64, + seqs []uint64, + fee std.Fee, + memo string, +) std.Tx { + t.Helper() + sigs := make([]std.Signature, len(privs)) for i, priv := range privs { - signBytes := std.SignBytes(chainID, accNums[i], seqs[i], fee, msgs, memo) + signBytes, err := std.GetSignaturePayload(std.SignDoc{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: seqs[i], + Fee: fee, + Msgs: msgs, + Memo: memo, + }) + require.NoError(t, err) sig, err := priv.Sign(signBytes) if err != nil { @@ -87,7 +126,7 @@ func NewTestTxWithMemo(chainID string, msgs []std.Msg, privs []crypto.PrivKey, a return tx } -func NewTestTxWithSignBytes(msgs []std.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee std.Fee, signBytes []byte, memo string) std.Tx { +func NewTestTxWithSignBytes(msgs []std.Msg, privs []crypto.PrivKey, fee std.Fee, signBytes []byte, memo string) std.Tx { sigs := make([]std.Signature, len(privs)) for i, priv := range privs { sig, err := priv.Sign(signBytes) diff --git a/tm2/pkg/std/doc.go b/tm2/pkg/std/doc.go index 92010e4136e..39958e8d46f 100644 --- a/tm2/pkg/std/doc.go +++ b/tm2/pkg/std/doc.go @@ -1,51 +1,47 @@ package std import ( - "time" + "fmt" "github.com/gnolang/gno/tm2/pkg/amino" "github.com/gnolang/gno/tm2/pkg/crypto" ) -//---------------------------------------- -// SignDoc - -// The standard object for all signing, including transactions and other -// documents. AccountNumber is a replay-prevention field for the whole account +// SignDoc is the standard object for transactions. +// AccountNumber is a replay-prevention field for the whole account // (eg. nonce) to prevent the replay of txs after an account has been deleted -// (due to zero balance). Time can also be used in the future instead of -// AccountNumber . Sequence is a replay-prevention field for each transaction -// given a nonce. +// (due to zero balance). Sequence is a replay-prevention field for each transaction +// given a nonce type SignDoc struct { - ChainID string `json:"chain_id" yaml:"chain_id"` - AccountNumber uint64 `json:"account_number" yaml:"account_number"` - Time time.Time `json:"time" yaml:"time"` - Sequence uint64 `json:"sequence" yaml:"sequence"` - Fee Fee `json:"fee" yaml:"fee"` - Msgs []Msg `json:"msgs" yaml:"msgs"` - Memo string `json:"memo" yaml:"memo"` + ChainID string `json:"chain_id" yaml:"chain_id"` + AccountNumber uint64 `json:"account_number" yaml:"account_number"` + Sequence uint64 `json:"sequence" yaml:"sequence"` + Fee Fee `json:"fee" yaml:"fee"` + Msgs []Msg `json:"msgs" yaml:"msgs"` + Memo string `json:"memo" yaml:"memo"` } -// SignBytes returns the bytes to sign for a transaction. -func SignBytes(chainID string, accountNumber uint64, sequence uint64, fee Fee, msgs []Msg, memo string) []byte { - bz, err := amino.MarshalJSON(SignDoc{ - ChainID: chainID, - AccountNumber: accountNumber, - Sequence: sequence, - Fee: fee, - Msgs: msgs, - Memo: memo, - }) +// GetSignaturePayload returns the sign payload for the SignDoc. +// The signature payload is generated by marshalling the SignDoc +// into Amino JSON, and then sorting the Amino JSON. +// Ultimately, the formula for signing is sign(sortJSON(aminoJSON(SignDoc))) +func GetSignaturePayload(s SignDoc) ([]byte, error) { + // Prepare the amino JSON + data, err := amino.MarshalJSON(s) if err != nil { - panic(err) + return nil, fmt.Errorf("unable to marshal sign doc, %w", err) + } + + // Sort the JSON + sortedData, err := sortJSON(data) + if err != nil { + return nil, fmt.Errorf("unable to sort payload JSON, %w", err) } - return MustSortJSON(bz) -} -//---------------------------------------- -// Signature + return sortedData, nil +} -// Signature represents a signature of a SignDoc. +// Signature represents a wrapped signature of a transaction type Signature struct { PubKey crypto.PubKey `json:"pub_key" yaml:"pub_key"` // optional Signature []byte `json:"signature" yaml:"signature"` diff --git a/tm2/pkg/std/doc_test.go b/tm2/pkg/std/doc_test.go new file mode 100644 index 00000000000..16f24c90cd2 --- /dev/null +++ b/tm2/pkg/std/doc_test.go @@ -0,0 +1,50 @@ +package std + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSignDoc_GetSignaturePayload(t *testing.T) { + t.Parallel() + + testTable := []struct { + name string + doc SignDoc + }{ + { + "empty sign doc", + SignDoc{}, + }, + { + "non-empty sign doc", + SignDoc{ + ChainID: "dummy", + AccountNumber: 10, + Sequence: 20, + Fee: Fee{ + GasFee: NewCoin("ugnot", 10), + GasWanted: 10, + }, + Msgs: []Msg{}, + Memo: "totally valid transaction", + }, + }, + } + + for _, testCase := range testTable { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + signPayload, err := GetSignaturePayload(testCase.doc) + require.NoError(t, err) + + assert.NotNil(t, signPayload) + assert.NotEmpty(t, signPayload) + }) + } +} diff --git a/tm2/pkg/std/tx.go b/tm2/pkg/std/tx.go index ae548f11d08..3068022ae03 100644 --- a/tm2/pkg/std/tx.go +++ b/tm2/pkg/std/tx.go @@ -99,8 +99,15 @@ func (tx Tx) GetMemo() string { return tx.Memo } // .Empty(). func (tx Tx) GetSignatures() []Signature { return tx.Signatures } -func (tx Tx) GetSignBytes(chainID string, accountNumber uint64, sequence uint64) []byte { - return SignBytes(chainID, accountNumber, sequence, tx.Fee, tx.Msgs, tx.Memo) +func (tx Tx) GetSignBytes(chainID string, accountNumber uint64, sequence uint64) ([]byte, error) { + return GetSignaturePayload(SignDoc{ + ChainID: chainID, + AccountNumber: accountNumber, + Sequence: sequence, + Fee: tx.Fee, + Msgs: tx.Msgs, + Memo: tx.Memo, + }) } // __________________________________________________________ diff --git a/tm2/pkg/std/utils.go b/tm2/pkg/std/utils.go index 98f23c803f8..df0847f20f5 100644 --- a/tm2/pkg/std/utils.go +++ b/tm2/pkg/std/utils.go @@ -2,28 +2,30 @@ package std import "encoding/json" -// SortedJSON takes any JSON and returns it sorted by keys. Also, all white-spaces +// sortJSON takes any JSON and returns it sorted by keys. Also, all white-spaces // are removed. // This method can be used to canonicalize JSON to be returned by GetSignBytes, // e.g. for the ledger integration. -// If the passed JSON isn't valid it will return an error. -func SortJSON(toSortJSON []byte) ([]byte, error) { - var c interface{} - err := json.Unmarshal(toSortJSON, &c) - if err != nil { +// If the passed JSON isn't valid it will return an error +func sortJSON(toSortJSON []byte) ([]byte, error) { + var c any + + if err := json.Unmarshal(toSortJSON, &c); err != nil { return nil, err } + js, err := json.Marshal(c) if err != nil { return nil, err } + return js, nil } -// MustSortJSON is like SortJSON but panic if an error occurs, e.g., if +// MustSortJSON is like sortJSON but panic if an error occurs, e.g., if // the passed JSON isn't valid. func MustSortJSON(toSortJSON []byte) []byte { - js, err := SortJSON(toSortJSON) + js, err := sortJSON(toSortJSON) if err != nil { panic(err) }