diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 5cc20185d4..de9a0253c5 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -328,7 +328,6 @@ func (tx testUpdatePowerTx) GetMsg() sdk.Msg { return tx func (tx testUpdatePowerTx) GetSignBytes() []byte { return nil } func (tx testUpdatePowerTx) ValidateBasic() sdk.Error { return nil } func (tx testUpdatePowerTx) GetSigners() []sdk.Address { return nil } -func (tx testUpdatePowerTx) GetFeePayer() sdk.Address { return nil } func (tx testUpdatePowerTx) GetSignatures() []sdk.StdSignature { return nil } func TestValidatorChange(t *testing.T) { diff --git a/client/builder/builder.go b/client/builder/builder.go index e252f40af8..a64dfeda74 100644 --- a/client/builder/builder.go +++ b/client/builder/builder.go @@ -124,7 +124,7 @@ func SignAndBuild(msg sdk.Msg, cdc *wire.Codec) ([]byte, error) { }} // marshal bytes - tx := sdk.NewStdTx(signMsg.Msg, sigs) + tx := sdk.NewStdTx(signMsg.Msg, signMsg.Fee, sigs) return cdc.MarshalBinary(tx) } diff --git a/docs/guide.md b/docs/guide.md index 1a6d21c82a..5c31d2e271 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -105,14 +105,6 @@ type Tx interface { GetMsg() Msg - // The address that pays the base fee for this message. The fee is - // deducted before the Msg is processed. - GetFeePayer() Address - - // Get the canonical byte representation of the Tx. - // Includes any signatures (or empty slots). - GetTxBytes() []byte - // Signatures returns the signature of signers who signed the Msg. // CONTRACT: Length returned is same as length of // pubkeys returned from MsgKeySigners, and the order @@ -148,8 +140,9 @@ case of Basecoin, the public key only needs to be included in the first transaction send by a given account - after that, the public key is forever stored by the application and can be left out of transactions. -Transactions can also specify the address responsible for paying the -transaction's fees using the `tx.GetFeePayer()` method. +The address responsible for paying the transactions fee is the first address +returned by msg.GetSigners(). The convenience function `FeePayer(tx Tx)` is provided +to return this. The standard way to create a transaction from a message is to use the `StdTx`: diff --git a/docs/sdk/overview.rst b/docs/sdk/overview.rst index bf7b23a606..9e79dd04ff 100644 --- a/docs/sdk/overview.rst +++ b/docs/sdk/overview.rst @@ -219,14 +219,6 @@ A transaction is a message with additional information for authentication: GetMsg() Msg - // The address that pays the base fee for this message. The fee is - // deducted before the Msg is processed. - GetFeePayer() Address - - // Get the canonical byte representation of the Tx. - // Includes any signatures (or empty slots). - GetTxBytes() []byte - // Signatures returns the signature of signers who signed the Msg. // CONTRACT: Length returned is same as length of // pubkeys returned from MsgKeySigners, and the order @@ -261,9 +253,6 @@ case of Basecoin, the public key only needs to be included in the first transaction send by a given account - after that, the public key is forever stored by the application and can be left out of transactions. -Transactions can also specify the address responsible for paying the -transaction's fees using the ``tx.GetFeePayer()`` method. - The standard way to create a transaction from a message is to use the ``StdTx``: :: diff --git a/examples/basecoin/app/app_test.go b/examples/basecoin/app/app_test.go index c2ef3e4547..66de2b68b4 100644 --- a/examples/basecoin/app/app_test.go +++ b/examples/basecoin/app/app_test.go @@ -29,6 +29,10 @@ var ( addr1 = priv1.PubKey().Address() addr2 = crypto.GenPrivKeyEd25519().PubKey().Address() coins = sdk.Coins{{"foocoin", 10}} + fee = sdk.StdFee{ + sdk.Coins{{"foocoin", 0}}, + 0, + } sendMsg = bank.SendMsg{ Inputs: []bank.Input{bank.NewInput(addr1, coins)}, @@ -82,8 +86,8 @@ func TestMsgs(t *testing.T) { sequences := []int64{0} for i, m := range msgs { - sig := priv1.Sign(sdk.StdSignBytes(chainID, sequences, m.msg)) - tx := sdk.NewStdTx(m.msg, []sdk.StdSignature{{ + sig := priv1.Sign(sdk.StdSignBytes(chainID, sequences, fee, m.msg)) + tx := sdk.NewStdTx(m.msg, fee, []sdk.StdSignature{{ PubKey: priv1.PubKey(), Signature: sig, }}) @@ -180,8 +184,8 @@ func TestSendMsgWithAccounts(t *testing.T) { // Sign the tx sequences := []int64{0} - sig := priv1.Sign(sdk.StdSignBytes(chainID, sequences, sendMsg)) - tx := sdk.NewStdTx(sendMsg, []sdk.StdSignature{{ + sig := priv1.Sign(sdk.StdSignBytes(chainID, sequences, fee, sendMsg)) + tx := sdk.NewStdTx(sendMsg, fee, []sdk.StdSignature{{ PubKey: priv1.PubKey(), Signature: sig, }}) @@ -213,7 +217,7 @@ func TestSendMsgWithAccounts(t *testing.T) { // resigning the tx with the bumped sequence should work sequences = []int64{1} - sig = priv1.Sign(sdk.StdSignBytes(chainID, sequences, tx.Msg)) + sig = priv1.Sign(sdk.StdSignBytes(chainID, sequences, fee, tx.Msg)) tx.Signatures[0].Signature = sig res = bapp.Deliver(tx) assert.Equal(t, sdk.CodeOK, res.Code, res.Log) @@ -270,9 +274,9 @@ func TestQuizMsg(t *testing.T) { func SignCheckDeliver(t *testing.T, bapp *BasecoinApp, msg sdk.Msg, seq int64, expPass bool) { // Sign the tx - tx := sdk.NewStdTx(msg, []sdk.StdSignature{{ + tx := sdk.NewStdTx(msg, fee, []sdk.StdSignature{{ PubKey: priv1.PubKey(), - Signature: priv1.Sign(sdk.StdSignBytes(chainID, []int64{seq}, msg)), + Signature: priv1.Sign(sdk.StdSignBytes(chainID, []int64{seq}, fee, msg)), Sequence: seq, }}) diff --git a/examples/basecoin/x/cool/types.go b/examples/basecoin/x/cool/types.go index f721bfa19b..10515c8abe 100644 --- a/examples/basecoin/x/cool/types.go +++ b/examples/basecoin/x/cool/types.go @@ -37,7 +37,7 @@ func (msg SetTrendMsg) String() string { // Validate Basic is used to quickly disqualify obviously invalid messages quickly func (msg SetTrendMsg) ValidateBasic() sdk.Error { if len(msg.Sender) == 0 { - return sdk.ErrUnrecognizedAddress(msg.Sender).Trace("") + return sdk.ErrUnrecognizedAddress(msg.Sender.String()).Trace("") } if strings.Contains(msg.Cool, "hot") { return sdk.ErrUnauthorized("").Trace("hot is not cool") @@ -88,7 +88,7 @@ func (msg QuizMsg) String() string { // Validate Basic is used to quickly disqualify obviously invalid messages quickly func (msg QuizMsg) ValidateBasic() sdk.Error { if len(msg.Sender) == 0 { - return sdk.ErrUnrecognizedAddress(msg.Sender).Trace("") + return sdk.ErrUnrecognizedAddress(msg.Sender.String()).Trace("") } return nil } diff --git a/examples/kvstore/tx.go b/examples/kvstore/tx.go index fdecf63807..c9c30c885d 100644 --- a/examples/kvstore/tx.go +++ b/examples/kvstore/tx.go @@ -51,10 +51,6 @@ func (tx kvstoreTx) GetSignatures() []sdk.StdSignature { return nil } -func (tx kvstoreTx) GetFeePayer() sdk.Address { - return nil -} - // takes raw transaction bytes and decodes them into an sdk.Tx. An sdk.Tx has // all the signatures and can be used to authenticate. func decodeTx(txBytes []byte) (sdk.Tx, sdk.Error) { diff --git a/mock/tx.go b/mock/tx.go index efe60feb91..326946eaa5 100644 --- a/mock/tx.go +++ b/mock/tx.go @@ -64,10 +64,6 @@ func (tx kvstoreTx) GetSignatures() []sdk.StdSignature { return nil } -func (tx kvstoreTx) GetFeePayer() sdk.Address { - return nil -} - // takes raw transaction bytes and decodes them into an sdk.Tx. An sdk.Tx has // all the signatures and can be used to authenticate. func decodeTx(txBytes []byte) (sdk.Tx, sdk.Error) { diff --git a/types/coin.go b/types/coin.go index ad541b99bd..92871cd179 100644 --- a/types/coin.go +++ b/types/coin.go @@ -139,8 +139,14 @@ func (coins Coins) IsGTE(coinsB Coins) bool { } // IsZero returns true if there are no coins +// or all coins are zero. func (coins Coins) IsZero() bool { - return len(coins) == 0 + for _, coin := range coins { + if !coin.IsZero() { + return false + } + } + return true } // IsEqual returns true if the two sets of Coins have the same value diff --git a/types/errors.go b/types/errors.go index 9d8175e309..48e08b1164 100644 --- a/types/errors.go +++ b/types/errors.go @@ -27,6 +27,7 @@ const ( CodeInsufficientFunds CodeType = 5 CodeUnknownRequest CodeType = 6 CodeUnrecognizedAddress CodeType = 7 + CodeInvalidPubKey CodeType = 8 CodeGenesisParse CodeType = 0xdead // TODO: remove ? ) @@ -50,6 +51,8 @@ func CodeToDefaultMsg(code CodeType) string { return "Unknown request" case CodeUnrecognizedAddress: return "Unrecognized address" + case CodeInvalidPubKey: + return "Invalid pubkey" default: return fmt.Sprintf("Unknown code %d", code) } @@ -81,8 +84,11 @@ func ErrInsufficientFunds(msg string) Error { func ErrUnknownRequest(msg string) Error { return newError(CodeUnknownRequest, msg) } -func ErrUnrecognizedAddress(addr Address) Error { - return newError(CodeUnrecognizedAddress, addr.String()) +func ErrUnrecognizedAddress(msg string) Error { + return newError(CodeUnrecognizedAddress, msg) +} +func ErrInvalidPubKey(msg string) Error { + return newError(CodeInvalidPubKey, msg) } //---------------------------------------- diff --git a/types/tx_msg.go b/types/tx_msg.go index 141f9d050d..21bc330540 100644 --- a/types/tx_msg.go +++ b/types/tx_msg.go @@ -33,10 +33,6 @@ type Tx interface { // Gets the Msg. GetMsg() Msg - // The address that pays the base fee for this message. The fee is - // deducted before the Msg is processed. - GetFeePayer() Address - // Signatures returns the signature of signers who signed the Msg. // CONTRACT: Length returned is same as length of // pubkeys returned from MsgKeySigners, and the order @@ -49,25 +45,60 @@ type Tx interface { var _ Tx = (*StdTx)(nil) -// StdTx is a standard way to wrap a Msg with Signatures. +// StdTx is a standard way to wrap a Msg with Fee and Signatures. // NOTE: the first signature is the FeePayer (Signatures must not be nil). type StdTx struct { Msg `json:"msg"` + Fee StdFee `json:"fee"` Signatures []StdSignature `json:"signatures"` } -func NewStdTx(msg Msg, sigs []StdSignature) StdTx { +func NewStdTx(msg Msg, fee StdFee, sigs []StdSignature) StdTx { return StdTx{ Msg: msg, + Fee: fee, Signatures: sigs, } } //nolint func (tx StdTx) GetMsg() Msg { return tx.Msg } -func (tx StdTx) GetFeePayer() Address { return tx.Signatures[0].PubKey.Address() } // XXX but PubKey is optional! func (tx StdTx) GetSignatures() []StdSignature { return tx.Signatures } +// FeePayer returns the address responsible for paying the fees +// for the transactions. It's the first address returned by msg.GetSigners(). +// If GetSigners() is empty, this panics. +func FeePayer(tx Tx) Address { + return tx.GetMsg().GetSigners()[0] +} + +//__________________________________________________________ + +// StdFee includes the amount of coins paid in fees and the maximum +// gas to be used by the transaction. The ratio yields an effective "gasprice", +// which must be above some miminum to be accepted into the mempool. +type StdFee struct { + Amount Coins `json"amount"` + Gas int64 `json"gas"` +} + +func NewStdFee(gas int64, amount ...Coin) StdFee { + return StdFee{ + Amount: amount, + Gas: gas, + } +} + +func (fee StdFee) Bytes() []byte { + bz, err := json.Marshal(fee) // TODO + if err != nil { + panic(err) + } + return bz +} + +//__________________________________________________________ + // StdSignDoc is replay-prevention structure. // It includes the result of msg.GetSignBytes(), // as well as the ChainID (prevent cross chain replay) @@ -76,27 +107,18 @@ func (tx StdTx) GetSignatures() []StdSignature { return tx.Signatures } type StdSignDoc struct { ChainID string `json:"chain_id"` Sequences []int64 `json:"sequences"` + FeeBytes []byte `json:"fee_bytes"` MsgBytes []byte `json:"msg_bytes"` - AltBytes []byte `json:"alt_bytes"` // TODO: do we really want this ? + AltBytes []byte `json:"alt_bytes"` } -// StdSignMsg is a convenience structure for passing along -// a Msg with the other requirements for a StdSignDoc before -// it is signed. For use in the CLI -type StdSignMsg struct { - ChainID string - Sequences []int64 - Msg Msg -} - -func (msg StdSignMsg) Bytes() []byte { - return StdSignBytes(msg.ChainID, msg.Sequences, msg.Msg) -} - -func StdSignBytes(chainID string, sequences []int64, msg Msg) []byte { +// StdSignBytes returns the bytes to sign for a transaction. +// TODO: change the API to just take a chainID and StdTx ? +func StdSignBytes(chainID string, sequences []int64, fee StdFee, msg Msg) []byte { bz, err := json.Marshal(StdSignDoc{ ChainID: chainID, Sequences: sequences, + FeeBytes: fee.Bytes(), MsgBytes: msg.GetSignBytes(), }) if err != nil { @@ -105,7 +127,51 @@ func StdSignBytes(chainID string, sequences []int64, msg Msg) []byte { return bz } -//------------------------------------- +// StdSignMsg is a convenience structure for passing along +// a Msg with the other requirements for a StdSignDoc before +// it is signed. For use in the CLI. +type StdSignMsg struct { + ChainID string + Sequences []int64 + Fee StdFee + Msg Msg + // XXX: Alt +} + +func (msg StdSignMsg) Bytes() []byte { + return StdSignBytes(msg.ChainID, msg.Sequences, msg.Fee, msg.Msg) +} + +//__________________________________________________________ // Application function variable used to unmarshal transaction bytes type TxDecoder func(txBytes []byte) (Tx, Error) + +//__________________________________________________________ + +var _ Msg = (*TestMsg)(nil) + +// msg type for testing +type TestMsg struct { + signers []Address +} + +func NewTestMsg(addrs ...Address) *TestMsg { + return &TestMsg{ + signers: addrs, + } +} + +func (msg *TestMsg) Type() string { return "TestMsg" } +func (msg *TestMsg) Get(key interface{}) (value interface{}) { return nil } +func (msg *TestMsg) GetSignBytes() []byte { + bz, err := json.Marshal(msg.signers) + if err != nil { + panic(err) + } + return bz +} +func (msg *TestMsg) ValidateBasic() Error { return nil } +func (msg *TestMsg) GetSigners() []Address { + return msg.signers +} diff --git a/types/tx_msg_test.go b/types/tx_msg_test.go new file mode 100644 index 0000000000..f72cdea26e --- /dev/null +++ b/types/tx_msg_test.go @@ -0,0 +1,30 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + crypto "github.com/tendermint/go-crypto" +) + +func newStdFee() StdFee { + return NewStdFee(100, + Coin{"atom", 150}, + ) +} + +func TestStdTx(t *testing.T) { + priv := crypto.GenPrivKeyEd25519() + addr := priv.PubKey().Address() + msg := NewTestMsg(addr) + fee := newStdFee() + sigs := []StdSignature{} + + tx := NewStdTx(msg, fee, sigs) + assert.Equal(t, msg, tx.GetMsg()) + assert.Equal(t, sigs, tx.GetSignatures()) + + feePayer := FeePayer(tx) + assert.Equal(t, addr, feePayer) +} diff --git a/x/auth/ante.go b/x/auth/ante.go index f2495af78d..43b9bb8341 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -1,12 +1,15 @@ package auth import ( + "bytes" "fmt" - "reflect" sdk "github.com/cosmos/cosmos-sdk/types" ) +// NewAnteHandler returns an AnteHandler that checks +// and increments sequence numbers, checks signatures, +// and deducts fees from the first signer. func NewAnteHandler(accountMapper sdk.AccountMapper) sdk.AnteHandler { return func( ctx sdk.Context, tx sdk.Tx, @@ -23,6 +26,12 @@ func NewAnteHandler(accountMapper sdk.AccountMapper) sdk.AnteHandler { // TODO: can tx just implement message? msg := tx.GetMsg() + // TODO: will this always be a stdtx? should that be used in the function signature? + stdTx, ok := tx.(sdk.StdTx) + if !ok { + return ctx, sdk.ErrInternal("tx must be sdk.StdTx").Result(), true + } + // Assert that number of signatures is correct. var signerAddrs = msg.GetSigners() if len(sigs) != len(signerAddrs) { @@ -31,51 +40,64 @@ func NewAnteHandler(accountMapper sdk.AccountMapper) sdk.AnteHandler { true } - // Collect accounts to set in the context - var signerAccs = make([]sdk.Account, len(signerAddrs)) - - // Get the sign bytes by collecting all sequence numbers + // Get the sign bytes (requires all sequence numbers and the fee) sequences := make([]int64, len(signerAddrs)) for i := 0; i < len(signerAddrs); i++ { sequences[i] = sigs[i].Sequence } - signBytes := sdk.StdSignBytes(ctx.ChainID(), sequences, msg) + fee := stdTx.Fee + signBytes := sdk.StdSignBytes(ctx.ChainID(), sequences, fee, msg) - // Check fee payer sig and nonce, and deduct fee. - // This is done first because it only - // requires fetching 1 account. - payerAddr, payerSig := signerAddrs[0], sigs[0] - payerAcc, res := processSig(ctx, accountMapper, payerAddr, payerSig, signBytes) - if !res.IsOK() { - return ctx, res, true - } - signerAccs[0] = payerAcc - // TODO: Charge fee from payerAcc. - // TODO: accountMapper.SetAccount(ctx, payerAddr) - - // Check sig and nonce for the rest. - for i := 1; i < len(sigs); i++ { + // Check sig and nonce and collect signer accounts. + var signerAccs = make([]sdk.Account, len(signerAddrs)) + for i := 0; i < len(sigs); i++ { signerAddr, sig := signerAddrs[i], sigs[i] - signerAcc, res := processSig(ctx, accountMapper, signerAddr, sig, signBytes) + + // check signature, return account with incremented nonce + signerAcc, res := processSig( + ctx, accountMapper, + signerAddr, sig, signBytes, + ) if !res.IsOK() { return ctx, res, true } + + // first sig pays the fees + if i == 0 { + // TODO: min fee + if !fee.Amount.IsZero() { + signerAcc, res = deductFees(signerAcc, fee) + if !res.IsOK() { + return ctx, res, true + } + } + } + + // Save the account. + accountMapper.SetAccount(ctx, signerAcc) signerAccs[i] = signerAcc } + // cache the signer accounts in the context ctx = WithSigners(ctx, signerAccs) + + // TODO: tx tags (?) + return ctx, sdk.Result{}, false // continue... } } // verify the signature and increment the sequence. -// if the account doesn't have a pubkey, set it as well. -func processSig(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, sig sdk.StdSignature, signBytes []byte) (acc sdk.Account, res sdk.Result) { +// if the account doesn't have a pubkey, set it. +func processSig( + ctx sdk.Context, am sdk.AccountMapper, + addr sdk.Address, sig sdk.StdSignature, signBytes []byte) ( + acc sdk.Account, res sdk.Result) { - // Get the account + // Get the account. acc = am.GetAccount(ctx, addr) if acc == nil { - return nil, sdk.ErrUnrecognizedAddress(addr).Result() + return nil, sdk.ErrUnrecognizedAddress(addr.String()).Result() } // Check and increment sequence number. @@ -86,20 +108,21 @@ func processSig(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, sig sdk } acc.SetSequence(seq + 1) - // Check and possibly set pubkey. + // If pubkey is not known for account, + // set it from the StdSignature. pubKey := acc.GetPubKey() if pubKey.Empty() { - if sig.PubKey.Empty() { - return nil, sdk.ErrInternal("public Key not found").Result() - } - if !reflect.DeepEqual(sig.PubKey.Address(), addr) { - return nil, sdk.ErrInternal( - fmt.Sprintf("invalid PubKey for address %v", addr)).Result() - } pubKey = sig.PubKey + if pubKey.Empty() { + return nil, sdk.ErrInvalidPubKey("PubKey not found").Result() + } + if !bytes.Equal(pubKey.Address(), addr) { + return nil, sdk.ErrInvalidPubKey( + fmt.Sprintf("PubKey does not match Signer address %v", addr)).Result() + } err := acc.SetPubKey(pubKey) if err != nil { - return nil, sdk.ErrInternal("setting PubKey on signer").Result() + return nil, sdk.ErrInternal("setting PubKey on signer's account").Result() } } // Check sig. @@ -107,7 +130,21 @@ func processSig(ctx sdk.Context, am sdk.AccountMapper, addr sdk.Address, sig sdk return nil, sdk.ErrUnauthorized("signature verification failed").Result() } - // Save the account. - am.SetAccount(ctx, acc) return } + +// Deduct the fee from the account. +// We could use the CoinKeeper (in addition to the AccountMapper, +// because the CoinKeeper doesn't give us accounts), but it seems easier to do this. +func deductFees(acc sdk.Account, fee sdk.StdFee) (sdk.Account, sdk.Result) { + coins := acc.GetCoins() + feeAmount := fee.Amount + + newCoins := coins.Minus(feeAmount) + if !newCoins.IsNotNegative() { + errMsg := fmt.Sprintf("%s < %s", coins, feeAmount) + return nil, sdk.ErrInsufficientFunds(errMsg).Result() + } + acc.SetCoins(newCoins) + return acc, sdk.Result{} +} diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index c3df2199d6..5bbb3d0b69 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -1,38 +1,32 @@ package auth import ( - "reflect" "testing" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" abci "github.com/tendermint/abci/types" crypto "github.com/tendermint/go-crypto" ) -// msg type for testing -type testMsg struct { - signBytes []byte - signers []sdk.Address +func newTestMsg(addrs ...sdk.Address) *sdk.TestMsg { + return sdk.NewTestMsg(addrs...) } -func newTestMsg(addrs ...sdk.Address) *testMsg { - return &testMsg{ - signBytes: []byte(addrs[0]), - signers: addrs, +func newStdFee() sdk.StdFee { + return sdk.NewStdFee(100, + sdk.Coin{"atom", 150}, + ) +} + +// coins to more than cover the fee +func newCoins() sdk.Coins { + return sdk.Coins{ + {"atom", 10000000}, } } -func (msg *testMsg) Type() string { return "testMsg" } -func (msg *testMsg) Get(key interface{}) (value interface{}) { return nil } -func (msg *testMsg) GetSignBytes() []byte { - return msg.signBytes -} -func (msg *testMsg) ValidateBasic() sdk.Error { return nil } -func (msg *testMsg) GetSigners() []sdk.Address { - return msg.signers -} - // generate a priv key and return it with its address func privAndAddr() (crypto.PrivKey, sdk.Address) { priv := crypto.GenPrivKeyEd25519() @@ -55,17 +49,18 @@ func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, assert.Equal(t, code, result.Code) } -func newTestTx(ctx sdk.Context, msg sdk.Msg, privs []crypto.PrivKey, seqs []int64) sdk.Tx { - signBytes := sdk.StdSignBytes(ctx.ChainID(), seqs, msg) - return newTestTxWithSignBytes(msg, privs, seqs, signBytes) +func newTestTx(ctx sdk.Context, msg sdk.Msg, privs []crypto.PrivKey, seqs []int64, fee sdk.StdFee) sdk.Tx { + signBytes := sdk.StdSignBytes(ctx.ChainID(), seqs, fee, msg) + return newTestTxWithSignBytes(msg, privs, seqs, fee, signBytes) } -func newTestTxWithSignBytes(msg sdk.Msg, privs []crypto.PrivKey, seqs []int64, signBytes []byte) sdk.Tx { +func newTestTxWithSignBytes(msg sdk.Msg, privs []crypto.PrivKey, seqs []int64, fee sdk.StdFee, signBytes []byte) sdk.Tx { sigs := make([]sdk.StdSignature, len(privs)) for i, priv := range privs { sigs[i] = sdk.StdSignature{PubKey: priv.PubKey(), Signature: priv.Sign(signBytes), Sequence: seqs[i]} } - return sdk.NewStdTx(msg, sigs) + tx := sdk.NewStdTx(msg, fee, sigs) + return tx } // Test various error cases in the AnteHandler control flow. @@ -83,21 +78,26 @@ func TestAnteHandlerSigErrors(t *testing.T) { // msg and signatures var tx sdk.Tx msg := newTestMsg(addr1, addr2) + fee := newStdFee() // test no signatures - tx = newTestTx(ctx, msg, []crypto.PrivKey{}, []int64{}) + privs, seqs := []crypto.PrivKey{}, []int64{} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) // test num sigs dont match GetSigners - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) + privs, seqs = []crypto.PrivKey{priv1}, []int64{0} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) // test an unrecognized account - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1, priv2}, []int64{0, 0}) + privs, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 0} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnrecognizedAddress) // save the first account, but second is still unrecognized acc1 := mapper.NewAccountWithAddress(ctx, addr1) + acc1.SetCoins(fee.Amount) mapper.SetAccount(ctx, acc1) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnrecognizedAddress) } @@ -116,28 +116,34 @@ func TestAnteHandlerSequences(t *testing.T) { // set the accounts acc1 := mapper.NewAccountWithAddress(ctx, addr1) + acc1.SetCoins(newCoins()) mapper.SetAccount(ctx, acc1) acc2 := mapper.NewAccountWithAddress(ctx, addr2) + acc2.SetCoins(newCoins()) mapper.SetAccount(ctx, acc2) // msg and signatures var tx sdk.Tx msg := newTestMsg(addr1) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) + fee := newStdFee() // test good tx from one signer + privs, seqs := []crypto.PrivKey{priv1}, []int64{0} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) // test sending it again fails (replay protection) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) // fix sequence, should pass - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{1}) + seqs = []int64{1} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) // new tx with another signer and correct sequences msg = newTestMsg(addr1, addr2) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1, priv2}, []int64{2, 0}) + privs, seqs = []crypto.PrivKey{priv1, priv2}, []int64{2, 0} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) // replay fails @@ -145,16 +151,54 @@ func TestAnteHandlerSequences(t *testing.T) { // tx from just second signer with incorrect sequence fails msg = newTestMsg(addr2) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv2}, []int64{0}) + privs, seqs = []crypto.PrivKey{priv2}, []int64{0} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) // fix the sequence and it passes - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv2}, []int64{1}) + tx = newTestTx(ctx, msg, []crypto.PrivKey{priv2}, []int64{1}, fee) checkValidTx(t, anteHandler, ctx, tx) // another tx from both of them that passes msg = newTestMsg(addr1, addr2) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1, priv2}, []int64{3, 2}) + privs, seqs = []crypto.PrivKey{priv1, priv2}, []int64{3, 2} + tx = newTestTx(ctx, msg, privs, seqs, fee) + checkValidTx(t, anteHandler, ctx, tx) +} + +// Test logic around fee deduction. +func TestAnteHandlerFees(t *testing.T) { + // setup + ms, capKey := setupMultiStore() + mapper := NewAccountMapper(capKey, &BaseAccount{}) + anteHandler := NewAnteHandler(mapper) + ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, false, nil) + + // keys and addresses + priv1, addr1 := privAndAddr() + + // set the accounts + acc1 := mapper.NewAccountWithAddress(ctx, addr1) + mapper.SetAccount(ctx, acc1) + + // msg and signatures + var tx sdk.Tx + msg := newTestMsg(addr1) + privs, seqs := []crypto.PrivKey{priv1}, []int64{0} + fee := sdk.NewStdFee(100, + sdk.Coin{"atom", 150}, + ) + + // signer does not have enough funds to pay the fee + tx = newTestTx(ctx, msg, privs, seqs, fee) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInsufficientFunds) + + acc1.SetCoins(sdk.Coins{{"atom", 149}}) + mapper.SetAccount(ctx, acc1) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInsufficientFunds) + + acc1.SetCoins(sdk.Coins{{"atom", 150}}) + mapper.SetAccount(ctx, acc1) checkValidTx(t, anteHandler, ctx, tx) } @@ -171,35 +215,63 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // set the accounts acc1 := mapper.NewAccountWithAddress(ctx, addr1) + acc1.SetCoins(newCoins()) mapper.SetAccount(ctx, acc1) acc2 := mapper.NewAccountWithAddress(ctx, addr2) + acc2.SetCoins(newCoins()) mapper.SetAccount(ctx, acc2) var tx sdk.Tx + msg := newTestMsg(addr1) + fee := newStdFee() + fee2 := newStdFee() + fee2.Gas += 100 + fee3 := newStdFee() + fee3.Amount[0].Amount += 100 // test good tx and signBytes - msg := newTestMsg(addr1) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) + privs, seqs := []crypto.PrivKey{priv1}, []int64{0} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) - // test invalid chain_id - tx = newTestTxWithSignBytes(msg, []crypto.PrivKey{priv1}, []int64{1}, sdk.StdSignBytes("", []int64{1}, msg)) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) - // test wrong seqs - tx = newTestTxWithSignBytes(msg, []crypto.PrivKey{priv1}, []int64{1}, sdk.StdSignBytes(ctx.ChainID(), []int64{2}, msg)) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) - // test wrong msg - tx = newTestTxWithSignBytes(msg, []crypto.PrivKey{priv1}, []int64{1}, sdk.StdSignBytes(ctx.ChainID(), []int64{1}, newTestMsg(addr2))) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) + chainID := ctx.ChainID() + chainID2 := chainID + "somemorestuff" + codeUnauth := sdk.CodeUnauthorized + + cases := []struct { + chainID string + seqs []int64 + fee sdk.StdFee + msg sdk.Msg + code sdk.CodeType + }{ + {chainID2, []int64{1}, fee, msg, codeUnauth}, // test wrong chain_id + {chainID, []int64{2}, fee, msg, codeUnauth}, // test wrong seqs + {chainID, []int64{1, 2}, fee, msg, codeUnauth}, // test wrong seqs + {chainID, []int64{1}, fee, newTestMsg(addr2), codeUnauth}, // test wrong msg + {chainID, []int64{1}, fee2, newTestMsg(addr2), codeUnauth}, // test wrong fee + {chainID, []int64{1}, fee3, newTestMsg(addr2), codeUnauth}, // test wrong fee + } + + privs, seqs = []crypto.PrivKey{priv1}, []int64{1} + for _, cs := range cases { + tx := newTestTxWithSignBytes( + msg, privs, seqs, fee, + sdk.StdSignBytes(cs.chainID, cs.seqs, cs.fee, cs.msg), + ) + checkInvalidTx(t, anteHandler, ctx, tx, cs.code) + } // test wrong signer if public key exist - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv2}, []int64{1}) + privs, seqs = []crypto.PrivKey{priv2}, []int64{1} + tx = newTestTx(ctx, msg, privs, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) // test wrong signer if public doesn't exist msg = newTestMsg(addr2) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInternal) + privs, seqs = []crypto.PrivKey{priv1}, []int64{0} + tx = newTestTx(ctx, msg, privs, seqs, fee) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) } @@ -216,33 +288,37 @@ func TestAnteHandlerSetPubKey(t *testing.T) { // set the accounts acc1 := mapper.NewAccountWithAddress(ctx, addr1) + acc1.SetCoins(newCoins()) mapper.SetAccount(ctx, acc1) acc2 := mapper.NewAccountWithAddress(ctx, addr2) + acc2.SetCoins(newCoins()) mapper.SetAccount(ctx, acc2) var tx sdk.Tx // test good tx and set public key msg := newTestMsg(addr1) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) + privs, seqs := []crypto.PrivKey{priv1}, []int64{0} + fee := newStdFee() + tx = newTestTx(ctx, msg, privs, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) acc1 = mapper.GetAccount(ctx, addr1) - reflect.DeepEqual(acc1.GetPubKey(), priv1.PubKey()) + require.Equal(t, acc1.GetPubKey(), priv1.PubKey()) // test public key not found msg = newTestMsg(addr2) - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) + tx = newTestTx(ctx, msg, privs, seqs, fee) sigs := tx.GetSignatures() sigs[0].PubKey = crypto.PubKey{} - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInternal) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) acc2 = mapper.GetAccount(ctx, addr2) assert.True(t, acc2.GetPubKey().Empty()) // test invalid signature and public key - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv1}, []int64{0}) - checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInternal) + tx = newTestTx(ctx, msg, privs, seqs, fee) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) acc2 = mapper.GetAccount(ctx, addr2) assert.True(t, acc2.GetPubKey().Empty()) diff --git a/x/auth/baseaccount_test.go b/x/auth/baseaccount_test.go index 85674af18b..b2f5b54ae2 100644 --- a/x/auth/baseaccount_test.go +++ b/x/auth/baseaccount_test.go @@ -11,42 +11,107 @@ import ( wire "github.com/cosmos/cosmos-sdk/wire" ) -func TestBaseAccount(t *testing.T) { +func keyPubAddr() (crypto.PrivKey, crypto.PubKey, sdk.Address) { key := crypto.GenPrivKeyEd25519() pub := key.PubKey() addr := pub.Address() + return key.Wrap(), pub, addr +} + +func TestBaseAccountAddressPubKey(t *testing.T) { + _, pub1, addr1 := keyPubAddr() + _, pub2, addr2 := keyPubAddr() + acc := NewBaseAccountWithAddress(addr1) + + // check the address (set) and pubkey (not set) + assert.EqualValues(t, addr1, acc.GetAddress()) + assert.EqualValues(t, crypto.PubKey{}, acc.GetPubKey()) + + // can't override address + err := acc.SetAddress(addr2) + assert.NotNil(t, err) + assert.EqualValues(t, addr1, acc.GetAddress()) + + // set the pubkey + err = acc.SetPubKey(pub1) + assert.Nil(t, err) + assert.Equal(t, pub1, acc.GetPubKey()) + + // can't override pubkey + err = acc.SetPubKey(pub2) + assert.NotNil(t, err) + assert.Equal(t, pub1, acc.GetPubKey()) + + //------------------------------------ + + // can set address on empty account + acc2 := BaseAccount{} + err = acc2.SetAddress(addr2) + assert.Nil(t, err) + assert.EqualValues(t, addr2, acc2.GetAddress()) +} + +func TestBaseAccountCoins(t *testing.T) { + _, _, addr := keyPubAddr() + acc := NewBaseAccountWithAddress(addr) + + someCoins := sdk.Coins{{"atom", 123}, {"eth", 246}} + + err := acc.SetCoins(someCoins) + assert.Nil(t, err) + assert.Equal(t, someCoins, acc.GetCoins()) +} + +func TestBaseAccountSequence(t *testing.T) { + _, _, addr := keyPubAddr() + acc := NewBaseAccountWithAddress(addr) + + seq := int64(7) + + err := acc.SetSequence(seq) + assert.Nil(t, err) + assert.Equal(t, seq, acc.GetSequence()) +} + +func TestBaseAccountMarshal(t *testing.T) { + _, pub, addr := keyPubAddr() + acc := NewBaseAccountWithAddress(addr) + someCoins := sdk.Coins{{"atom", 123}, {"eth", 246}} seq := int64(7) - acc := NewBaseAccountWithAddress(addr) + // set everything on the account + err := acc.SetPubKey(pub) + assert.Nil(t, err) + err = acc.SetSequence(seq) + assert.Nil(t, err) + err = acc.SetCoins(someCoins) + assert.Nil(t, err) // need a codec for marshaling codec := wire.NewCodec() wire.RegisterCrypto(codec) - err := acc.SetPubKey(pub) - assert.Nil(t, err) - assert.Equal(t, pub, acc.GetPubKey()) - - assert.EqualValues(t, addr, acc.GetAddress()) - - err = acc.SetCoins(someCoins) - assert.Nil(t, err) - assert.Equal(t, someCoins, acc.GetCoins()) - - err = acc.SetSequence(seq) - assert.Nil(t, err) - assert.Equal(t, seq, acc.GetSequence()) - b, err := codec.MarshalBinary(acc) assert.Nil(t, err) - var acc2 BaseAccount + acc2 := BaseAccount{} err = codec.UnmarshalBinary(b, &acc2) assert.Nil(t, err) assert.Equal(t, acc, acc2) + // error on bad bytes acc2 = BaseAccount{} err = codec.UnmarshalBinary(b[:len(b)/2], &acc2) assert.NotNil(t, err) + +} + +func TestBaseAccountGetSet(t *testing.T) { + _, _, addr := keyPubAddr() + acc := NewBaseAccountWithAddress(addr) + + // Get/Set are not yet defined - all values cause a panic. + assert.Panics(t, func() { acc.Get("key") }) + assert.Panics(t, func() { acc.Set("key", "value") }) } diff --git a/x/auth/context.go b/x/auth/context.go index 90de99f783..91259d9e6d 100644 --- a/x/auth/context.go +++ b/x/auth/context.go @@ -38,5 +38,9 @@ func WithSigners(ctx types.Context, accounts []types.Account) types.Context { } func GetSigners(ctx types.Context) []types.Account { - return ctx.Value(contextKeySigners).([]types.Account) + v := ctx.Value(contextKeySigners) + if v == nil { + return []types.Account{} + } + return v.([]types.Account) } diff --git a/x/auth/context_test.go b/x/auth/context_test.go new file mode 100644 index 0000000000..0e4db8b080 --- /dev/null +++ b/x/auth/context_test.go @@ -0,0 +1,39 @@ +package auth + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + abci "github.com/tendermint/abci/types" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func TestContextWithSigners(t *testing.T) { + ms, _ := setupMultiStore() + ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, false, nil) + + _, _, addr1 := keyPubAddr() + _, _, addr2 := keyPubAddr() + acc1 := NewBaseAccountWithAddress(addr1) + acc1.SetSequence(7132) + acc2 := NewBaseAccountWithAddress(addr2) + acc2.SetSequence(8821) + + // new ctx has no signers + signers := GetSigners(ctx) + assert.Equal(t, 0, len(signers)) + + ctx2 := WithSigners(ctx, []sdk.Account{&acc1, &acc2}) + + // original context is unchanged + signers = GetSigners(ctx) + assert.Equal(t, 0, len(signers)) + + // new context has signers + signers = GetSigners(ctx2) + assert.Equal(t, 2, len(signers)) + assert.Equal(t, acc1, *(signers[0].(*BaseAccount))) + assert.Equal(t, acc2, *(signers[1].(*BaseAccount))) +} diff --git a/x/bank/mapper.go b/x/bank/mapper.go index 76e7f4e2f5..ab2e854202 100644 --- a/x/bank/mapper.go +++ b/x/bank/mapper.go @@ -20,7 +20,7 @@ func NewCoinKeeper(am sdk.AccountMapper) CoinKeeper { func (ck CoinKeeper) SubtractCoins(ctx sdk.Context, addr sdk.Address, amt sdk.Coins) (sdk.Coins, sdk.Error) { acc := ck.am.GetAccount(ctx, addr) if acc == nil { - return amt, sdk.ErrUnrecognizedAddress(addr) + return amt, sdk.ErrUnrecognizedAddress(addr.String()) } coins := acc.GetCoins()