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

feat: add tx fee event #9860

Merged
merged 9 commits into from
Aug 9, 2021
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* [\#9860](https://github.com/cosmos/cosmos-sdk/pull/9860) Emit transaction fee in ante handler fee decorator. The event type is `tx` and the attribute is `fee`.
* [\#9776](https://github.com/cosmos/cosmos-sdk/pull/9776) Add flag `staking-bond-denom` to specify the staking bond denomination value when initializing a new chain.
* [\#9533](https://github.com/cosmos/cosmos-sdk/pull/9533) Added a new gRPC method, `DenomOwners`, in `x/bank` to query for all account holders of a specific denomination.
* (bank) [\#9618](https://github.com/cosmos/cosmos-sdk/pull/9618) Update bank.Metadata: add URI and URIHash attributes.
Expand Down
1 change: 1 addition & 0 deletions types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ var (

AttributeKeyAccountSequence = "acc_seq"
AttributeKeySignature = "signature"
AttributeKeyFee = "fee"

EventTypeMessage = "message"

Expand Down
5 changes: 5 additions & 0 deletions x/auth/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo
}
}

events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx,
sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR Colin!

This is the total fee payed by the tx, represented as a coin/denom?
I'm trying to test this with Hermes and will provide feedback on how easy is it for the relayer to use the fee info.

As a side-note, I'm trying to rebuild my local gaiad using this branch, and was wondering if there any instructions for doing that. I'm patching my gaia go.mod but not sure that's the right approach.

Copy link
Contributor Author

@colin-axner colin-axner Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is the total fee, but the fee may be multiple coins. Each coin would be represented by denom/amount. An empty coin set returns an empty string

Using the SDK's simd binary is another alternative to gaiad. You can run make build or make install depending on your usage

Edit: I realize SDK's simd doesn't have IBC

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the entire fee. In the case of the Hub, it would be Nuatom, but could be multiple coins as @colin-axner pointed out, e.g. Nuatom,Yphoton

)}
ctx.EventManager().EmitEvents(events)

return next(ctx, tx, simulate)
}

Expand Down
70 changes: 70 additions & 0 deletions x/auth/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,76 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmdByEvents() {
}
}

func (s *IntegrationTestSuite) TestCLIQueryTxsCmdByEvents() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took this from the QueryTxCmdByEvents and swapped out the command being executed, test cases, and the logic when checking the happy case

val := s.network.Validators[0]

account2, err := val.ClientCtx.Keyring.Key("newAccount2")
s.Require().NoError(err)

sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 10)

// Send coins.
out, err := s.createBankMsg(
val, account2.GetAddress(),
sdk.NewCoins(sendTokens),
)
s.Require().NoError(err)
var txRes sdk.TxResponse
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes))
s.Require().NoError(s.network.WaitForNextBlock())

// Query the tx by hash to get the inner tx.
out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, authcli.QueryTxCmd(), []string{txRes.TxHash, fmt.Sprintf("--%s=json", tmcli.OutputFlag)})
s.Require().NoError(err)
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes))

testCases := []struct {
name string
args []string
expectEmpty bool
}{
{
"fee event happy case",
[]string{
fmt.Sprintf("--events=tx.fee=%s",
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
},
false,
},
{
"no matching fee event",
[]string{
fmt.Sprintf("--events=tx.fee=%s",
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0))).String()),
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
},
true,
},
}

for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
cmd := authcli.QueryTxsByEventsCmd()
clientCtx := val.ClientCtx

out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
s.Require().NoError(err)

var result sdk.SearchTxsResult
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &result))

if tc.expectEmpty {
s.Require().Equal(0, len(result.Txs))
} else {
s.Require().NotEqual(0, len(result.Txs))
s.Require().NotNil(result.Txs[0])
}
})
}
}

func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() {
val1 := s.network.Validators[0]

Expand Down