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(e2e): fix e2e test for consensus min gas fee #4244 #4317

Merged
Merged
Show file tree
Hide file tree
Changes from 5 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
26 changes: 26 additions & 0 deletions tests/e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,32 @@ This section contains common "gotchas" that is sometimes very good to know when

To fix that, check the docker container and see if you node is working. Or remove all related container then rerun e2e test

3. Transaction fees.

Consensus min fee is set to "0.0025". This is how much is charged for 1 unit of gas. By default, we specify the gas limit
of `40000`. Therefore, each transaction should take `fee = 0.0025 uosmo / gas * 400000 gas = 1000 fee token.
The fee denom is set in `tests/e2e/initialization/config.go` by the value of `E2EFeeToken`.
See "Hermes Relayer - Consensus Min Fee" section for the relevant relayer configuration.
## Hermes Relayer
The configuration is built using the script in `tests/e2e/scripts/hermes_bootstrap.sh`
Repository: <https://github.dev/informalsystems/hermes>
### Consensus Min Fee
We set the following parameters Hermes configs to enable the consensus min fee in Osmosis:
- `gas_price` - Specifies the price per gas used of the fee to submit a transaction and
the denomination of the fee. The specified gas price should always be greater or equal to the `min-gas-price`
configured on the chain. This is to ensure that at least some minimal price is
paid for each unit of gas per transaction.
In Osmosis, we set consensus min fee = .0025 uosmo / gas * 400000 gas = 1000
See ConsensusMinFee in x/txfees/types/constants.go
- `default_gas` - the gas amount to use when simulation fails.
## Debugging
This section contains information about debugging osmosis's `e2e` tests.
Expand Down
16 changes: 14 additions & 2 deletions tests/e2e/configurer/chain/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,30 @@ func (c *Config) SendIBC(dstChain *Config, recipient string, token sdk.Coin) {
dstNode, err := dstChain.GetDefaultNode()
require.NoError(c.t, err)

balancesDstPre, err := dstNode.QueryBalances(recipient)
// removes the fee token from balances for calculating the difference in other tokens
// before and after the IBC send.
removeFeeTokenFromBalance := func(balance sdk.Coins) sdk.Coins {
feeTokenBalance := balance.FilterDenoms([]string{initialization.E2EFeeToken})
return balance.Sub(feeTokenBalance)
}

balancesDstPreWithTxFeeBalance, err := dstNode.QueryBalances(recipient)
require.NoError(c.t, err)

balancesDstPre := removeFeeTokenFromBalance(balancesDstPreWithTxFeeBalance)

cmd := []string{"hermes", "tx", "raw", "ft-transfer", dstChain.Id, c.Id, "transfer", "channel-0", token.Amount.String(), fmt.Sprintf("--denom=%s", token.Denom), fmt.Sprintf("--receiver=%s", recipient), "--timeout-height-offset=1000"}
_, _, err = c.containerManager.ExecHermesCmd(c.t, cmd, "Success")
require.NoError(c.t, err)

require.Eventually(
c.t,
func() bool {
balancesDstPost, err := dstNode.QueryBalances(recipient)
balancesDstPostWithTxFeeBalance, err := dstNode.QueryBalances(recipient)
require.NoError(c.t, err)

balancesDstPost := removeFeeTokenFromBalance(balancesDstPostWithTxFeeBalance)

ibcCoin := balancesDstPost.Sub(balancesDstPre)
if ibcCoin.Len() == 1 {
tokenPre := balancesDstPre.AmountOfNoDenomValidation(ibcCoin[0].Denom)
Expand Down
9 changes: 9 additions & 0 deletions tests/e2e/configurer/config/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,13 @@ var (
InitialMinDeposit = MinDepositValue / 4
// Minimum expedited deposit value for proposal to be submitted.
InitialMinExpeditedDeposit = MinExpeditedDepositValue / 4
// The first id of a pool create via CLI before starting an
// upgrade.
// Note: that we create a pool with id 1 via genesis
// in the initialization package. As a result, the first
// pre-upgrade should have id 2.
// This value gets mutated during the pre-upgrade pool
// creation in case more pools are added to genesis
// in the future
PreUpgradePoolId uint64 = 2
)
11 changes: 6 additions & 5 deletions tests/e2e/configurer/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,20 @@ func (uc *UpgradeConfigurer) CreatePreUpgradeState() error {
chainA.SendIBC(chainB, chainB.NodeConfigs[0].PublicAddress, initialization.StakeToken)
chainB.SendIBC(chainA, chainA.NodeConfigs[0].PublicAddress, initialization.StakeToken)

chainANode.CreateBalancerPool("pool1A.json", initialization.ValidatorWalletName)
config.PreUpgradePoolId = chainANode.CreateBalancerPool("pool1A.json", initialization.ValidatorWalletName)
poolShareDenom := fmt.Sprintf("gamm/pool/%d", config.PreUpgradePoolId)
chainBNode.CreateBalancerPool("pool1B.json", initialization.ValidatorWalletName)

// enable superfluid assets on chainA
chainA.EnableSuperfluidAsset("gamm/pool/1")
chainA.EnableSuperfluidAsset(poolShareDenom)

// setup wallets and send gamm tokens to these wallets (only chainA)
lockupWalletAddrA, lockupWalletSuperfluidAddrA := chainANode.CreateWallet(lockupWallet), chainANode.CreateWallet(lockupWalletSuperfluid)
chainANode.BankSend("10000000000000000000gamm/pool/1", chainA.NodeConfigs[0].PublicAddress, lockupWalletAddrA)
chainANode.BankSend("10000000000000000000gamm/pool/1", chainA.NodeConfigs[0].PublicAddress, lockupWalletSuperfluidAddrA)
chainANode.BankSend("10000000000000000000"+poolShareDenom, chainA.NodeConfigs[0].PublicAddress, lockupWalletAddrA)
chainANode.BankSend("10000000000000000000"+poolShareDenom, chainA.NodeConfigs[0].PublicAddress, lockupWalletSuperfluidAddrA)

// test lock and add to existing lock for both regular and superfluid lockups (only chainA)
chainA.LockAndAddToExistingLock(sdk.NewInt(1000000000000000000), "gamm/pool/1", lockupWalletAddrA, lockupWalletSuperfluidAddrA)
chainA.LockAndAddToExistingLock(sdk.NewInt(1000000000000000000), poolShareDenom, lockupWalletAddrA, lockupWalletSuperfluidAddrA)

return nil
}
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/containers/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ const (
// It should be uploaded to Docker Hub. OSMOSIS_E2E_SKIP_UPGRADE should be unset
// for this functionality to be used.
previousVersionOsmoRepository = "osmolabs/osmosis-dev"
previousVersionOsmoTag = "v14.x-6b742c84-1675172347"
previousVersionOsmoTag = "v14.x-4d4583fa-1676370337"
// Pre-upgrade repo/tag for osmosis initialization (this should be one version below upgradeVersion)
previousVersionInitRepository = "osmolabs/osmosis-e2e-init-chain"
previousVersionInitTag = "v14.x-f6813bcb-1674140667-manual"
previousVersionInitTag = "v14.x-4d4583fa-1676370337-manual"
// Hermes repo/version for relayer
relayerRepository = "osmolabs/hermes"
relayerTag = "0.13.0"
Expand Down
20 changes: 16 additions & 4 deletions tests/e2e/containers/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,32 @@ import (
"github.com/ory/dockertest/v3/docker"
"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/osmosis-labs/osmosis/v14/tests/e2e/initialization"
txfeestypes "github.com/osmosis-labs/osmosis/v14/x/txfees/types"
)

const (
hermesContainerName = "hermes-relayer"
// The maximum number of times debug logs are printed to console
// per CLI command.
maxDebugLogsPerCommand = 3

GasLimit = 400000
)

var defaultErrRegex = regexp.MustCompile(`(E|e)rror`)
var (
// We set consensus min fee = .0025 uosmo / gas * 400000 gas = 1000
Fees = txfeestypes.ConsensusMinFee.Mul(sdk.NewDec(GasLimit)).Ceil().TruncateInt64()

defaultErrRegex = regexp.MustCompile(`(E|e)rror`)

txArgs = []string{"-b=block", "--yes", "--keyring-backend=test", "--log_format=json"}

// See ConsensusMinFee in x/txfees/types/constants.go
txDefaultGasArgs = []string{fmt.Sprintf("--gas=%d", GasLimit), fmt.Sprintf("--fees=%d", Fees) + initialization.E2EFeeToken}
)

// Manager is a wrapper around all Docker instances, and the Docker API.
// It provides utilities to run and interact with all Docker containers used within e2e testing.
Expand Down Expand Up @@ -60,9 +75,6 @@ func (m *Manager) ExecTxCmd(t *testing.T, chainId string, containerName string,
return m.ExecTxCmdWithSuccessString(t, chainId, containerName, command, "code: 0")
}

var txArgs = []string{"-b=block", "--yes", "--keyring-backend=test", "--log_format=json"}
var txDefaultGasArgs = []string{"--gas=400000", "--fees=1000" + initialization.E2EFeeToken}

// ExecTxCmdWithSuccessString Runs ExecCmd, with flags for txs added.
// namely adding flags `--chain-id={chain-id} -b=block --yes --keyring-backend=test "--log_format=json" --gas=400000`,
// and searching for `successStr`
Expand Down
9 changes: 5 additions & 4 deletions tests/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ func (s *IntegrationTestSuite) TestGeometricTwapMigration() {
const (
// Configurations for tests/e2e/scripts/pool1A.json
// This pool gets initialized pre-upgrade.
oldPoolId = 1
minAmountOut = "1"
otherDenom = "ibc/ED07A3391A112B175915CD8FAF43A2DA8E4790EDE12566649D0C2F97716B8518"
migrationWallet = "migration"
Expand All @@ -175,7 +174,7 @@ func (s *IntegrationTestSuite) TestGeometricTwapMigration() {
node.BankSend(uosmoIn, chainA.NodeConfigs[0].PublicAddress, swapWalletAddr)

// Swap to create new twap records on the pool that was created pre-upgrade.
node.SwapExactAmountIn(uosmoIn, minAmountOut, fmt.Sprintf("%d", oldPoolId), otherDenom, swapWalletAddr)
node.SwapExactAmountIn(uosmoIn, minAmountOut, fmt.Sprintf("%d", config.PreUpgradePoolId), otherDenom, swapWalletAddr)
}

// TestIBCTokenTransfer tests that IBC token transfers work as expected.
Expand Down Expand Up @@ -516,9 +515,11 @@ func (s *IntegrationTestSuite) TestAddToExistingLockPostUpgrade() {
s.NoError(err)
// ensure we can add to existing locks and superfluid locks that existed pre upgrade on chainA
// we use the hardcoded gamm/pool/1 and these specific wallet names to match what was created pre upgrade
preUpgradePoolShareDenom := fmt.Sprintf("gamm/pool/%d", config.PreUpgradePoolId)

lockupWalletAddr, lockupWalletSuperfluidAddr := chainANode.GetWallet("lockup-wallet"), chainANode.GetWallet("lockup-wallet-superfluid")
chainANode.AddToExistingLock(sdk.NewInt(1000000000000000000), "gamm/pool/1", "240s", lockupWalletAddr)
chainANode.AddToExistingLock(sdk.NewInt(1000000000000000000), "gamm/pool/1", "240s", lockupWalletSuperfluidAddr)
chainANode.AddToExistingLock(sdk.NewInt(1000000000000000000), preUpgradePoolShareDenom, "240s", lockupWalletAddr)
chainANode.AddToExistingLock(sdk.NewInt(1000000000000000000), preUpgradePoolShareDenom, "240s", lockupWalletSuperfluidAddr)
}

// TestAddToExistingLock tests lockups to both regular and superfluid locks.
Expand Down
6 changes: 4 additions & 2 deletions tests/e2e/scripts/hermes_bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ account_prefix = 'osmo'
key_name = 'val01-osmosis-a'
store_prefix = 'ibc'
max_gas = 6000000
gas_price = { price = 0.000, denom = 'uosmo' }
default_gas = 400000
gas_price = { price = 0.0025, denom = 'e2e-default-feetoken' }
gas_adjustment = 1.0
clock_drift = '1m' # to accomdate docker containers
trusting_period = '239seconds'
Expand All @@ -57,7 +58,8 @@ account_prefix = 'osmo'
key_name = 'val01-osmosis-b'
store_prefix = 'ibc'
max_gas = 6000000
gas_price = { price = 0.000, denom = 'uosmo' }
default_gas = 400000
Copy link
Member

Choose a reason for hiding this comment

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

Why are we putting default gas? Won't we get out of gas errors routinely?

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIU, this is what the relayer assumes for calculating fees when estimation fails ref: informalsystems/hermes#1457

We use the value of 40k in the tx arguments as well so I just reused it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, the only remaining problem is the geometric twap failing for reasons I cannot currently explain. Please let me know if something immediately comes to mind as the solution.

I'm going to keep investigating in the meantime

gas_price = { price = 0.0025, denom = 'e2e-default-feetoken' }
gas_adjustment = 1.0
clock_drift = '1m' # to accomdate docker containers
trusting_period = '239seconds'
Expand Down