Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize auth/type.StdFee #4509

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .pending/breaking/sdk/4509-New-Fee-generic
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#4509 New `Fee` generic interface available in the top level types package.
`auth.StdFee` implements such interface. User defined auth module can now
define their own custom fee types.
8 changes: 4 additions & 4 deletions client/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ func TestConfiguredTxEncoder(t *testing.T) {
}

func TestReadStdTxFromFile(t *testing.T) {
cdc := codec.New()
sdk.RegisterCodec(cdc)
cdc := makeCodec()

// Build a test transaction
fee := authtypes.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)})
stdTx := authtypes.NewStdTx([]sdk.Msg{}, fee, nil, "foomemo")

// Write it to the file
encodedTx, _ := cdc.MarshalJSON(stdTx)
encodedTx, err := cdc.MarshalJSON(stdTx)
require.NoError(t, err)
jsonTxFile := writeToNewTempFile(t, string(encodedTx))
defer os.Remove(jsonTxFile.Name())

Expand Down Expand Up @@ -158,7 +158,7 @@ func TestValidateCmd(t *testing.T) {

func compareEncoders(t *testing.T, expected sdk.TxEncoder, actual sdk.TxEncoder) {
msgs := []sdk.Msg{sdk.NewTestMsg(addr)}
tx := authtypes.NewStdTx(msgs, authtypes.StdFee{}, []sdk.Signature{}, "")
tx := authtypes.NewStdTx(msgs, sdk.Fee(nil), []sdk.Signature{}, "")

defaultEncoderBytes, err := expected(tx)
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions docs/spec/auth/03_types.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ a struct which implements the `sdk.Tx` interface using `StdFee` and `StdSignatur

## StdFee

A `StdFee` is simply the combination of a fee amount, in any number of denominations,
and a gas limit (where dividing the amount by the gas limit gives a "gas price").
`StdFee` implements the `sdk.Fee` interface and consists of a combination of a fee
amount, in any number of denominations, and a gas limit (where dividing the amount
by the gas limit gives a "gas price").

```golang
type StdFee struct {
Expand Down
1 change: 1 addition & 0 deletions types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ func RegisterCodec(cdc *codec.Codec) {
cdc.RegisterInterface((*Msg)(nil), nil)
cdc.RegisterInterface((*Tx)(nil), nil)
cdc.RegisterInterface((*Signature)(nil), nil)
cdc.RegisterInterface((*Fee)(nil), nil)
}
10 changes: 10 additions & 0 deletions types/tx_msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ type Signature interface {
GetSignature() []byte
}

//__________________________________________________________

// Fee defines the properties of a fee concrete type.
type Fee interface {
GasLimit() uint64
Cost() Coins
Bytes() []byte
GasPrices() DecCoins
}

// TxDecoder unmarshals transaction bytes
type TxDecoder func(txBytes []byte) (Tx, Error)

Expand Down
28 changes: 14 additions & 14 deletions x/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si
}
}

newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas)
newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.GasLimit())

// AnteHandlers must have their own defer/recover in order for the BaseApp
// to know how much gas was used! This is because the GasMeter is created in
Expand All @@ -73,11 +73,11 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si
case sdk.ErrorOutOfGas:
log := fmt.Sprintf(
"out of gas in location: %v; gasWanted: %d, gasUsed: %d",
rType.Descriptor, stdTx.Fee.Gas, newCtx.GasMeter().GasConsumed(),
rType.Descriptor, stdTx.Fee.GasLimit(), newCtx.GasMeter().GasConsumed(),
)
res = sdk.ErrOutOfGas(log).Result()

res.GasWanted = stdTx.Fee.Gas
res.GasWanted = stdTx.Fee.GasLimit()
res.GasUsed = newCtx.GasMeter().GasConsumed()
abort = true
default:
Expand Down Expand Up @@ -112,13 +112,13 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si
return newCtx, res, true
}

if !stdTx.Fee.Amount.IsZero() {
if !stdTx.Fee.Cost().IsZero() {
signerAccs[0], res = DeductFees(ctx.BlockHeader().Time, signerAccs[0], stdTx.Fee)
if !res.IsOK() {
return newCtx, res, true
}

fck.AddCollectedFees(newCtx, stdTx.Fee.Amount)
fck.AddCollectedFees(newCtx, stdTx.Fee.Cost())
}

// stdSigs contains the sequence number, account number, and signatures.
Expand Down Expand Up @@ -146,7 +146,7 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper, sigGasConsumer Si
}

// TODO: tx tags (?)
return newCtx, sdk.Result{GasWanted: stdTx.Fee.Gas}, false // continue...
return newCtx, sdk.Result{GasWanted: stdTx.Fee.GasLimit()}, false // continue...
}
}

Expand Down Expand Up @@ -328,17 +328,17 @@ func consumeMultisignatureVerificationGas(meter sdk.GasMeter,
//
// NOTE: We could use the CoinKeeper (in addition to the AccountKeeper, because
// the CoinKeeper doesn't give us accounts), but it seems easier to do this.
func DeductFees(blockTime time.Time, acc Account, fee StdFee) (Account, sdk.Result) {
func DeductFees(blockTime time.Time, acc Account, fee sdk.Fee) (Account, sdk.Result) {
coins := acc.GetCoins()
feeAmount := fee.Amount
feeAmount := fee.Cost()

if !feeAmount.IsValid() {
return nil, sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee amount: %s", feeAmount)).Result()
}

// get the resulting coins deducting the fees
newCoins, ok := coins.SafeSub(feeAmount)
if ok {
newCoins, insufficientFunds := coins.SafeSub(feeAmount)
if insufficientFunds {
return nil, sdk.ErrInsufficientFunds(
fmt.Sprintf("insufficient funds to pay for fees; %s < %s", coins, feeAmount),
).Result()
Expand Down Expand Up @@ -366,23 +366,23 @@ func DeductFees(blockTime time.Time, acc Account, fee StdFee) (Account, sdk.Resu
//
// Contract: This should only be called during CheckTx as it cannot be part of
// consensus.
func EnsureSufficientMempoolFees(ctx sdk.Context, stdFee StdFee) sdk.Result {
func EnsureSufficientMempoolFees(ctx sdk.Context, stdFee sdk.Fee) sdk.Result {
minGasPrices := ctx.MinGasPrices()
if !minGasPrices.IsZero() {
requiredFees := make(sdk.Coins, len(minGasPrices))

// Determine the required fees by multiplying each required minimum gas
// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
glDec := sdk.NewDec(int64(stdFee.Gas))
glDec := sdk.NewDec(int64(stdFee.GasLimit()))
for i, gp := range minGasPrices {
fee := gp.Amount.Mul(glDec)
requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
}

if !stdFee.Amount.IsAnyGTE(requiredFees) {
if !stdFee.Cost().IsAnyGTE(requiredFees) {
return sdk.ErrInsufficientFee(
fmt.Sprintf(
"insufficient fees; got: %q required: %q", stdFee.Amount, requiredFees,
"insufficient fees; got: %q required: %q", stdFee.Cost(), requiredFees,
),
).Result()
}
Expand Down
4 changes: 2 additions & 2 deletions x/auth/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context,
stdTx, ok := tx.(types.StdTx)
require.True(t, ok, "tx must be in form auth.types.StdTx")
// GasWanted set correctly
require.Equal(t, stdTx.Fee.Gas, result.GasWanted, "Gas wanted not set correctly")
require.Equal(t, stdTx.Fee.GasLimit(), result.GasWanted, "Gas wanted not set correctly")
require.True(t, result.GasUsed > result.GasWanted, "GasUsed not greated than GasWanted")
// Check that context is set correctly
require.Equal(t, result.GasUsed, newCtx.GasMeter().GasConsumed(), "Context not updated correctly")
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestAnteHandlerSigErrors(t *testing.T) {

// save the first account, but second is still unrecognized
acc1 := input.ak.NewAccountWithAddress(ctx, addr1)
acc1.SetCoins(fee.Amount)
acc1.SetCoins(fee.Cost())
input.ak.SetAccount(ctx, acc1)
checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnknownAddress)
}
Expand Down
1 change: 1 addition & 0 deletions x/auth/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func RegisterCodec(cdc *codec.Codec) {
cdc.RegisterConcrete(&DelayedVestingAccount{}, "auth/DelayedVestingAccount", nil)
cdc.RegisterConcrete(StdTx{}, "auth/StdTx", nil)
cdc.RegisterConcrete(StdSignature{}, "auth/StdSignature", nil)
cdc.RegisterConcrete(StdFee{}, "auth/StdFee", nil)
}

// RegisterBaseAccount most users shouldn't use this, but this comes in handy for tests.
Expand Down
2 changes: 1 addition & 1 deletion x/auth/types/stdsignmsg.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type StdSignMsg struct {
ChainID string `json:"chain_id"`
AccountNumber uint64 `json:"account_number"`
Sequence uint64 `json:"sequence"`
Fee StdFee `json:"fee"`
Fee sdk.Fee `json:"fee"`
Msgs []sdk.Msg `json:"msgs"`
Memo string `json:"memo"`
}
Expand Down
23 changes: 15 additions & 8 deletions x/auth/types/stdtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
var (
_ sdk.Tx = (*StdTx)(nil)
_ sdk.Signature = (*StdSignature)(nil)
_ sdk.Fee = (*StdFee)(nil)

maxGasWanted = uint64((1 << 63) - 1)
)
Expand All @@ -22,12 +23,12 @@ var (
// NOTE: the first signature is the fee payer (Signatures must not be nil).
type StdTx struct {
Msgs []sdk.Msg `json:"msg"`
Fee StdFee `json:"fee"`
Fee sdk.Fee `json:"fee"`
Signatures []sdk.Signature `json:"signatures"`
Memo string `json:"memo"`
}

func NewStdTx(msgs []sdk.Msg, fee StdFee, sigs []sdk.Signature, memo string) StdTx {
func NewStdTx(msgs []sdk.Msg, fee sdk.Fee, sigs []sdk.Signature, memo string) StdTx {
return StdTx{
Msgs: msgs,
Fee: fee,
Expand All @@ -44,11 +45,11 @@ func (tx StdTx) GetMsgs() []sdk.Msg { return tx.Msgs }
func (tx StdTx) ValidateBasic() sdk.Error {
stdSigs := tx.GetSignatures()

if tx.Fee.Gas > maxGasWanted {
return sdk.ErrGasOverflow(fmt.Sprintf("invalid gas supplied; %d > %d", tx.Fee.Gas, maxGasWanted))
if tx.Fee.GasLimit() > maxGasWanted {
return sdk.ErrGasOverflow(fmt.Sprintf("invalid gas supplied; %d > %d", tx.Fee.GasLimit(), maxGasWanted))
}
if tx.Fee.Amount.IsAnyNegative() {
return sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee %s amount provided", tx.Fee.Amount))
if tx.Fee.Cost().IsAnyNegative() {
return sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee %s amount provided", tx.Fee.Cost()))
}
if len(stdSigs) == 0 {
return sdk.ErrNoSignatures("no signers")
Expand Down Expand Up @@ -117,14 +118,20 @@ type StdFee struct {
Gas uint64 `json:"gas"`
}

// NewStdFee returns a new instance of StdFee
// NewStdFee returns a new instance of StdFee.
func NewStdFee(gas uint64, amount sdk.Coins) StdFee {
return StdFee{
Amount: amount,
Gas: gas,
}
}

// GasLimit returns the maximum gas that can be used by a transaction.
func (fee StdFee) GasLimit() uint64 { return fee.Gas }

// Cost returns the amount of coins paid in fees.
func (fee StdFee) Cost() sdk.Coins { return fee.Amount }

// Bytes for signing later
func (fee StdFee) Bytes() []byte {
// normalize. XXX
Expand Down Expand Up @@ -167,7 +174,7 @@ type StdSignDoc struct {
}

// StdSignBytes returns the bytes to sign for a transaction.
func StdSignBytes(chainID string, accnum uint64, sequence uint64, fee StdFee, msgs []sdk.Msg, memo string) []byte {
func StdSignBytes(chainID string, accnum uint64, sequence uint64, fee sdk.Fee, msgs []sdk.Msg, memo string) []byte {
var msgsBytes []json.RawMessage
for _, msg := range msgs {
msgsBytes = append(msgsBytes, json.RawMessage(msg.GetSignBytes()))
Expand Down
17 changes: 13 additions & 4 deletions x/auth/types/stdtx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"testing"

"github.com/stretchr/testify/require"

abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/ed25519"
Expand Down Expand Up @@ -39,7 +38,7 @@ func TestStdSignBytes(t *testing.T) {
chainID string
accnum uint64
sequence uint64
fee StdFee
fee sdk.Fee
msgs []sdk.Msg
memo string
}
Expand All @@ -50,7 +49,7 @@ func TestStdSignBytes(t *testing.T) {
}{
{
args{"1234", 3, 6, defaultFee, []sdk.Msg{sdk.NewTestMsg(addr)}, "memo"},
fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"50000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}", addr),
fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"type":"auth/StdFee","value":{"amount":[{"amount":"150","denom":"atom"}],"gas":"50000"}},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr),
},
}
for i, tc := range tests {
Expand All @@ -74,7 +73,7 @@ func TestTxValidateBasic(t *testing.T) {

// require to fail validation upon invalid fee
badFee := NewTestStdFee()
badFee.Amount[0].Amount = sdk.NewInt(-5)
badFee.Cost()[0].Amount = sdk.NewInt(-5)
tx := NewTestTx(ctx, nil, nil, nil, nil, badFee)

err := tx.ValidateBasic()
Expand Down Expand Up @@ -143,3 +142,13 @@ func TestNewStdSignature(t *testing.T) {
require.True(t, bytes.Equal(got.GetSignature(), sigBytes))
require.True(t, got.GetPubKey().Equals(pub))
}

func TestNewStdFee(t *testing.T) {
amount := sdk.NewCoins(sdk.NewInt64Coin("test", 10))
fee := NewStdFee(100, amount)
require.True(t, fee.Cost().IsEqual(amount))
require.Equal(t, fee.GasLimit(), uint64(100))
gasPrices, err := sdk.ParseDecCoins("0.1test")
require.NoError(t, err)
require.True(t, gasPrices.IsEqual(fee.GasPrices()))
}
6 changes: 3 additions & 3 deletions x/auth/types/test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func KeyTestPubAddr() (crypto.PrivKey, crypto.PubKey, sdk.AccAddress) {
return key, pub, addr
}

func NewTestTx(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee StdFee) sdk.Tx {
func NewTestTx(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee sdk.Fee) sdk.Tx {
sigs := make([]sdk.Signature, len(privs))
for i, priv := range privs {
signBytes := StdSignBytes(ctx.ChainID(), accNums[i], seqs[i], fee, msgs, "")
Expand All @@ -86,7 +86,7 @@ func NewTestTx(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums
return tx
}

func NewTestTxWithMemo(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee StdFee, memo string) sdk.Tx {
func NewTestTxWithMemo(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee sdk.Fee, memo string) sdk.Tx {
sigs := make([]sdk.Signature, len(privs))
for i, priv := range privs {
signBytes := StdSignBytes(ctx.ChainID(), accNums[i], seqs[i], fee, msgs, memo)
Expand All @@ -103,7 +103,7 @@ func NewTestTxWithMemo(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey,
return tx
}

func NewTestTxWithSignBytes(msgs []sdk.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee StdFee, signBytes []byte, memo string) sdk.Tx {
func NewTestTxWithSignBytes(msgs []sdk.Msg, privs []crypto.PrivKey, accNums []uint64, seqs []uint64, fee sdk.Fee, signBytes []byte, memo string) sdk.Tx {
sigs := make([]sdk.Signature, len(privs))
for i, priv := range privs {
sig, err := priv.Sign(signBytes)
Expand Down
4 changes: 2 additions & 2 deletions x/genutil/genesis_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestValidateGenesisMultipleMessages(t *testing.T) {
msg2 := staking.NewMsgCreateValidator(sdk.ValAddress(pk2.Address()), pk2,
sdk.NewInt64Coin(sdk.DefaultBondDenom, 50), desc, comm, sdk.OneInt())

genTxs := auth.NewStdTx([]sdk.Msg{msg1, msg2}, auth.StdFee{}, nil, "")
genTxs := auth.NewStdTx([]sdk.Msg{msg1, msg2}, sdk.Fee(nil), nil, "")
genesisState := NewGenesisStateFromStdTx([]auth.StdTx{genTxs})

err := ValidateGenesis(genesisState)
Expand All @@ -39,7 +39,7 @@ func TestValidateGenesisBadMessage(t *testing.T) {

msg1 := staking.NewMsgEditValidator(sdk.ValAddress(pk1.Address()), desc, nil, nil)

genTxs := auth.NewStdTx([]sdk.Msg{msg1}, auth.StdFee{}, nil, "")
genTxs := auth.NewStdTx([]sdk.Msg{msg1}, sdk.Fee(nil), nil, "")
genesisState := NewGenesisStateFromStdTx([]auth.StdTx{genTxs})

err := ValidateGenesis(genesisState)
Expand Down
6 changes: 1 addition & 5 deletions x/mock/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,7 @@ func SetGenesis(app *App, accs []auth.Account) {
// GenTx generates a signed mock transaction.
func GenTx(msgs []sdk.Msg, accnums []uint64, seq []uint64, priv ...crypto.PrivKey) auth.StdTx {
// Make the transaction free
fee := auth.StdFee{
Amount: sdk.NewCoins(sdk.NewInt64Coin("foocoin", 0)),
Gas: 100000,
}

fee := auth.NewStdFee(100000, sdk.NewCoins(sdk.NewInt64Coin("foocoin", 0)))
sigs := make([]sdk.Signature, len(priv))
memo := "testmemotestmemo"

Expand Down