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

fix: eliminate fee double-charge by using configurable decorator #9051

Merged
merged 5 commits into from
Mar 11, 2024
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
74 changes: 74 additions & 0 deletions a3p-integration/proposals/a:upgrade-next/ante-fees.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import test from 'ava';

import { CHAINID, GOV1ADDR, GOV2ADDR, agd } from '@agoric/synthetic-chain';

test(`ante handler sends fee only to vbank/reserve`, async t => {
const [feeCollector, vbankReserve] = await Promise.all(
michaelfig marked this conversation as resolved.
Show resolved Hide resolved
// Look up addresses for fee collector and reserve accounts.
['fee_collector', 'vbank/reserve'].map(async name => {
const {
account: {
'@type': moduleAcct,
base_account: { address },
},
} = await agd.query('auth', 'module-account', name);

t.is(
moduleAcct,
'/cosmos.auth.v1beta1.ModuleAccount',
`${name} is a module account`,
);
return address;
}),
);

const getBalances = addresses =>
Promise.all(
addresses.map(async address => {
const { balances } = await agd.query('bank', 'balances', address);
return balances;
}),
);

const [feeCollectorStartBalances, vbankReserveStartBalances] =
await getBalances([feeCollector, vbankReserve]);

// Send a transaction with a known fee.
const feeAmount = 999n;
const feeDenom = 'uist';
const result = await agd.tx(
`bank send ${GOV1ADDR} ${GOV2ADDR} 1234ubld --fees=${feeAmount}${feeDenom} \
--from=${GOV1ADDR} --chain-id=${CHAINID} --keyring-backend=test --yes`,
);
michaelfig marked this conversation as resolved.
Show resolved Hide resolved
t.like(result, { code: 0 });

const [feeCollectorEndBalances, vbankReserveEndBalances] = await getBalances([
feeCollector,
vbankReserve,
]);
t.deepEqual(feeCollectorEndBalances, feeCollectorStartBalances);

michaelfig marked this conversation as resolved.
Show resolved Hide resolved
// The reserve balances should have increased by exactly the fee (possibly
// from zero, in which case start balances wouldn't include its denomination).
const feeDenomIndex = vbankReserveStartBalances.findIndex(
({ denom }) => denom === feeDenom,
);
const preFeeAmount =
feeDenomIndex < 0
? 0n
: BigInt(vbankReserveStartBalances[feeDenomIndex].amount);
const beforeCount =
feeDenomIndex < 0 ? vbankReserveStartBalances.length : feeDenomIndex;

const vbankReserveExpectedBalances = [
...vbankReserveStartBalances.slice(0, beforeCount),
{ amount: String(preFeeAmount + feeAmount), denom: feeDenom },
...vbankReserveStartBalances.slice(beforeCount + 1),
];

t.deepEqual(
vbankReserveEndBalances,
vbankReserveExpectedBalances,
'vbank/reserve should receive the fee',
);
});
5 changes: 1 addition & 4 deletions golang/cosmos/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,12 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
anteDecorators := []sdk.AnteDecorator{
ante.NewSetUpContextDecorator(),
ante.NewExtensionOptionsDecorator(nil), // reject all extensions
// former ante.NewMempoolFeeDecorator()
// replaced as in https://github.com/provenance-io/provenance/pull/1016
ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, nil),
ante.NewValidateBasicDecorator(),
ante.NewTxTimeoutHeightDecorator(),
ante.NewValidateMemoDecorator(opts.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
NewInboundDecorator(opts.SwingsetKeeper),
NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.FeeCollectorName),
ante.NewDeductFeeDecoratorWithName(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, nil, opts.FeeCollectorName),
// SetPubKeyDecorator must be called before all signature verification decorators
ante.NewSetPubKeyDecorator(opts.AccountKeeper),
ante.NewValidateSigCountDecorator(opts.AccountKeeper),
Expand Down
97 changes: 0 additions & 97 deletions golang/cosmos/ante/fee.go

This file was deleted.

2 changes: 1 addition & 1 deletion golang/cosmos/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ replace (
// Agoric-specific replacements:
replace (
// We need a fork of cosmos-sdk until all of the differences are merged.
github.com/cosmos/cosmos-sdk => github.com/agoric-labs/cosmos-sdk v0.46.16-alpha.agoric.2
github.com/cosmos/cosmos-sdk => github.com/agoric-labs/cosmos-sdk v0.46.16-alpha.agoric.2.1
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the new tag be v0.46.16-alpha.agoric.3? I feel like I'm missing something.

Copy link
Member

Choose a reason for hiding this comment

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

.3 was already taken by future incompatible changes (waiting in the wings of an agoric-sdk PR, but I'm not sure which one). .2.1 implies the first published revision of .2, which is more accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't use the current tip of Agoric since lien support was removed in agoric-labs/cosmos-sdk#385 (in accordance with #8988, which is on master but not cherry-picked into the upgrade-14 release branch). So we need to use a side branch of Agoric for the new fee decorator feature. Our naming convention is to use <cosmos-release>-alpha.agoric.N sequentially on Agoric, ...-alpha.agoric.N.M for sub-branches, and presumably ...N.M.P for sub-sub-branches, etc. I'll document the policy in that repo.


// Async version negotiation
github.com/cosmos/ibc-go/v6 => github.com/agoric-labs/ibc-go/v6 v6.2.1-alpha.agoric.3
Expand Down
4 changes: 2 additions & 2 deletions golang/cosmos/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBA
github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c=
github.com/agoric-labs/cometbft v0.34.30-alpha.agoric.1 h1:tqCNL72pQXdUmBzgv1md5SN2U3K/PaYQ4qZ5pFv8v6w=
github.com/agoric-labs/cometbft v0.34.30-alpha.agoric.1/go.mod h1:myvkihZD8eg9jKE3WFaugkNoL5nvEqlP7Jbjg98pCek=
github.com/agoric-labs/cosmos-sdk v0.46.16-alpha.agoric.2 h1:iHHqpYC0JzMbH4UYnQrcwVjLyHJuQphB0ogHbuLz44c=
github.com/agoric-labs/cosmos-sdk v0.46.16-alpha.agoric.2/go.mod h1:zUe5lsg/X7SeSO1nGkzOh9EGKO295szfrxIxYmeLYic=
github.com/agoric-labs/cosmos-sdk v0.46.16-alpha.agoric.2.1 h1:VZFX9Mogwt4cVTnkdt9zA6UJue4XYXdBURNhlTWw71Q=
github.com/agoric-labs/cosmos-sdk v0.46.16-alpha.agoric.2.1/go.mod h1:zUe5lsg/X7SeSO1nGkzOh9EGKO295szfrxIxYmeLYic=
github.com/agoric-labs/cosmos-sdk/ics23/go v0.8.0-alpha.agoric.1 h1:2jvHI/2d+psWAZy6FQ0vXJCHUtfU3ZbbW+pQFL04arQ=
github.com/agoric-labs/cosmos-sdk/ics23/go v0.8.0-alpha.agoric.1/go.mod h1:E45NqnlpxGnpfTWL/xauN7MRwEE28T4Dd4uraToOaKg=
github.com/agoric-labs/ibc-go/v6 v6.2.1-alpha.agoric.3 h1:YqvVwK+Lg/ZsuwyVm9UbPs8K55fg00R3Y9KnmaTBdgc=
Expand Down
8 changes: 4 additions & 4 deletions scripts/generate-a3p-submission-dir.sh
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/bin/bash
set -ueo pipefail

SCRIPT_DIR=$( cd ${0%/*} && pwd -P )
SCRIPT_DIR=$( cd "${0%/*}" && pwd -P )

for proposal in ./proposals/?:*
do
cd $proposal
args=`jq -r < package.json '.agoricProposal["sdk-generate"][0]'`
$SCRIPT_DIR/generate-a3p-submission.sh $proposal $args
cd "$proposal"
args=$(jq -r < package.json '.agoricProposal["sdk-generate"][0]')
"$SCRIPT_DIR/generate-a3p-submission.sh" "$proposal" $args
cd -
done
Loading