diff --git a/PENDING.md b/PENDING.md index a905c1939..fbb089f95 100644 --- a/PENDING.md +++ b/PENDING.md @@ -10,6 +10,7 @@ BREAKING CHANGES * Gaia - [#128](https://github.com/tendermint/devops/issues/128) Updated CircleCI job to trigger website build on every push to master/develop. * SDK + - [auth] \#2952 Signatures are no longer serialized on chain with the account number and sequence number * Tendermint diff --git a/cmd/gaia/app/benchmarks/txsize_test.go b/cmd/gaia/app/benchmarks/txsize_test.go index 24789b10e..a401c16ca 100644 --- a/cmd/gaia/app/benchmarks/txsize_test.go +++ b/cmd/gaia/app/benchmarks/txsize_test.go @@ -12,10 +12,11 @@ import ( // This will fail half the time with the second output being 173 // This is due to secp256k1 signatures not being constant size. -// This will be resolved when updating to tendermint v0.24.0 // nolint: vet func ExampleTxSendSize() { cdc := app.MakeCodec() + var gas uint64 = 1 + priv1 := secp256k1.GenPrivKeySecp256k1([]byte{0}) addr1 := sdk.AccAddress(priv1.PubKey().Address()) priv2 := secp256k1.GenPrivKeySecp256k1([]byte{1}) @@ -25,11 +26,14 @@ func ExampleTxSendSize() { Inputs: []bank.Input{bank.NewInput(addr1, coins)}, Outputs: []bank.Output{bank.NewOutput(addr2, coins)}, } - sig, _ := priv1.Sign(msg1.GetSignBytes()) - sigs := []auth.StdSignature{{nil, sig, 0, 0}} - tx := auth.NewStdTx([]sdk.Msg{msg1}, auth.NewStdFee(0, coins...), sigs, "") + fee := auth.NewStdFee(gas, coins...) + signBytes := auth.StdSignBytes("example-chain-ID", + 1, 1, fee, []sdk.Msg{msg1}, "") + sig, _ := priv1.Sign(signBytes) + sigs := []auth.StdSignature{{nil, sig}} + tx := auth.NewStdTx([]sdk.Msg{msg1}, fee, sigs, "") fmt.Println(len(cdc.MustMarshalBinaryBare([]sdk.Msg{msg1}))) fmt.Println(len(cdc.MustMarshalBinaryBare(tx))) // output: 80 - // 167 + // 169 } diff --git a/x/auth/ante.go b/x/auth/ante.go index 685d60949..0b3c2fd6b 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -77,17 +77,14 @@ func NewAnteHandler(am AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler { // When simulating, this would just be a 0-length slice. stdSigs := stdTx.GetSignatures() signerAddrs := stdTx.GetSigners() - - // create the list of all sign bytes - signBytesList := getSignBytesList(newCtx.ChainID(), stdTx, stdSigs) signerAccs, res := getSignerAccs(newCtx, am, signerAddrs) if !res.IsOK() { return newCtx, res, true } - res = validateAccNumAndSequence(ctx, signerAccs, stdSigs) - if !res.IsOK() { - return newCtx, res, true - } + + isGenesis := ctx.BlockHeight() == 0 + // create the list of all sign bytes + signBytesList := getSignBytesList(newCtx.ChainID(), stdTx, signerAccs, isGenesis) // first sig pays the fees if !stdTx.Fee.Amount.IsZero() { @@ -129,31 +126,6 @@ func getSignerAccs(ctx sdk.Context, am AccountKeeper, addrs []sdk.AccAddress) (a return } -func validateAccNumAndSequence(ctx sdk.Context, accs []Account, sigs []StdSignature) sdk.Result { - for i := 0; i < len(accs); i++ { - // On InitChain, make sure account number == 0 - if ctx.BlockHeight() == 0 && sigs[i].AccountNumber != 0 { - return sdk.ErrInvalidSequence( - fmt.Sprintf("Invalid account number for BlockHeight == 0. Got %d, expected 0", sigs[i].AccountNumber)).Result() - } - - // Check account number. - accnum := accs[i].GetAccountNumber() - if ctx.BlockHeight() != 0 && accnum != sigs[i].AccountNumber { - return sdk.ErrInvalidSequence( - fmt.Sprintf("Invalid account number. Got %d, expected %d", sigs[i].AccountNumber, accnum)).Result() - } - - // Check sequence number. - seq := accs[i].GetSequence() - if seq != sigs[i].Sequence { - return sdk.ErrInvalidSequence( - fmt.Sprintf("Invalid sequence. Got %d, expected %d", sigs[i].Sequence, seq)).Result() - } - } - return sdk.Result{} -} - // verify the signature and increment the sequence. // if the account doesn't have a pubkey, set it. func processSig(ctx sdk.Context, @@ -297,11 +269,15 @@ func setGasMeter(simulate bool, ctx sdk.Context, stdTx StdTx) sdk.Context { return ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) } -func getSignBytesList(chainID string, stdTx StdTx, stdSigs []StdSignature) (signatureBytesList [][]byte) { - signatureBytesList = make([][]byte, len(stdSigs)) - for i := 0; i < len(stdSigs); i++ { +func getSignBytesList(chainID string, stdTx StdTx, accs []Account, genesis bool) (signatureBytesList [][]byte) { + signatureBytesList = make([][]byte, len(accs)) + for i := 0; i < len(accs); i++ { + accNum := accs[i].GetAccountNumber() + if genesis { + accNum = 0 + } signatureBytesList[i] = StdSignBytes(chainID, - stdSigs[i].AccountNumber, stdSigs[i].Sequence, + accNum, accs[i].GetSequence(), stdTx.Fee, stdTx.Msgs, stdTx.Memo) } return diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 8d7118e74..88a63094a 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -74,7 +74,7 @@ func newTestTx(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums if err != nil { panic(err) } - sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: sig, AccountNumber: accNums[i], Sequence: seqs[i]} + sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: sig} } tx := NewStdTx(msgs, fee, sigs, "") return tx @@ -88,7 +88,7 @@ func newTestTxWithMemo(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, if err != nil { panic(err) } - sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: sig, AccountNumber: accNums[i], Sequence: seqs[i]} + sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: sig} } tx := NewStdTx(msgs, fee, sigs, memo) return tx @@ -102,7 +102,7 @@ func newTestTxWithSignBytes(msgs []sdk.Msg, privs []crypto.PrivKey, accNums []ui if err != nil { panic(err) } - sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: sig, AccountNumber: accNums[i], Sequence: seqs[i]} + sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: sig} } tx := NewStdTx(msgs, fee, sigs, memo) return tx @@ -200,7 +200,7 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { // new tx from wrong account number seqs = []uint64{1} tx = newTestTx(ctx, msgs, privs, []uint64{1}, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // from correct account number seqs = []uint64{1} @@ -213,7 +213,7 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { msgs = []sdk.Msg{msg1, msg2} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{1, 0}, []uint64{2, 0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // correct account numbers privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{2, 0} @@ -260,7 +260,7 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { // new tx from wrong account number seqs = []uint64{1} tx = newTestTx(ctx, msgs, privs, []uint64{1}, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // from correct account number seqs = []uint64{1} @@ -273,7 +273,7 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { msgs = []sdk.Msg{msg1, msg2} privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{1, 0}, []uint64{2, 0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // correct account numbers privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []uint64{0, 0}, []uint64{2, 0} @@ -322,7 +322,7 @@ func TestAnteHandlerSequences(t *testing.T) { checkValidTx(t, anteHandler, ctx, tx, false) // test sending it again fails (replay protection) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // fix sequence, should pass seqs = []uint64{1} @@ -339,14 +339,14 @@ func TestAnteHandlerSequences(t *testing.T) { checkValidTx(t, anteHandler, ctx, tx, false) // replay fails - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // tx from just second signer with incorrect sequence fails msg = newTestMsg(addr2) msgs = []sdk.Msg{msg} privs, accnums, seqs = []crypto.PrivKey{priv2}, []uint64{1}, []uint64{0} tx = newTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidSequence) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) // fix the sequence and it passes tx = newTestTx(ctx, msgs, []crypto.PrivKey{priv2}, []uint64{1}, []uint64{1}, fee) diff --git a/x/auth/client/txbuilder/txbuilder.go b/x/auth/client/txbuilder/txbuilder.go index 8c0ce232b..0adc76b64 100644 --- a/x/auth/client/txbuilder/txbuilder.go +++ b/x/auth/client/txbuilder/txbuilder.go @@ -162,9 +162,7 @@ func (bldr TxBuilder) BuildWithPubKey(name string, msgs []sdk.Msg) ([]byte, erro } sigs := []auth.StdSignature{{ - AccountNumber: msg.AccountNumber, - Sequence: msg.Sequence, - PubKey: info.GetPubKey(), + PubKey: info.GetPubKey(), }} return bldr.Codec.MarshalBinaryLengthPrefixed(auth.NewStdTx(msg.Msgs, msg.Fee, sigs, msg.Memo)) @@ -206,9 +204,7 @@ func MakeSignature(name, passphrase string, msg StdSignMsg) (sig auth.StdSignatu return } return auth.StdSignature{ - AccountNumber: msg.AccountNumber, - Sequence: msg.Sequence, - PubKey: pubkey, - Signature: sigBytes, + PubKey: pubkey, + Signature: sigBytes, }, nil } diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index 2f8ae4158..59edf7c13 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -188,8 +188,6 @@ func StdSignBytes(chainID string, accnum uint64, sequence uint64, fee StdFee, ms type StdSignature struct { crypto.PubKey `json:"pub_key"` // optional Signature []byte `json:"signature"` - AccountNumber uint64 `json:"account_number"` - Sequence uint64 `json:"sequence"` } // logic for standard transaction decoding diff --git a/x/bank/app_test.go b/x/bank/app_test.go index 128705b58..42796d643 100644 --- a/x/bank/app_test.go +++ b/x/bank/app_test.go @@ -123,7 +123,7 @@ func TestMsgSendWithAccounts(t *testing.T) { msgs: []sdk.Msg{sendMsg1, sendMsg2}, accNums: []uint64{0}, accSeqs: []uint64{0}, - expSimPass: false, + expSimPass: true, // doesn't check signature expPass: false, privKeys: []crypto.PrivKey{priv1}, }, @@ -136,19 +136,6 @@ func TestMsgSendWithAccounts(t *testing.T) { mock.CheckBalance(t, mapp, eb.addr, eb.coins) } } - - // bumping the tx nonce number without resigning should be an auth error - mapp.BeginBlock(abci.RequestBeginBlock{}) - - tx := mock.GenTx([]sdk.Msg{sendMsg1}, []uint64{0}, []uint64{0}, priv1) - tx.Signatures[0].Sequence = 1 - - res := mapp.Deliver(tx) - require.EqualValues(t, sdk.CodeUnauthorized, res.Code, res.Log) - require.EqualValues(t, sdk.CodespaceRoot, res.Codespace) - - // resigning the tx with the bumped sequence should work - mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{sendMsg1, sendMsg2}, []uint64{0}, []uint64{1}, true, true, priv1) } func TestMsgSendMultipleOut(t *testing.T) { diff --git a/x/mock/app.go b/x/mock/app.go index 6382ba05b..9251a95c0 100644 --- a/x/mock/app.go +++ b/x/mock/app.go @@ -202,10 +202,8 @@ func GenTx(msgs []sdk.Msg, accnums []uint64, seq []uint64, priv ...crypto.PrivKe } sigs[i] = auth.StdSignature{ - PubKey: p.PubKey(), - Signature: sig, - AccountNumber: accnums[i], - Sequence: seq[i], + PubKey: p.PubKey(), + Signature: sig, } }