From 17fff370f868d124026e76442ad52ad7c76782f6 Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 05:46:32 +0200 Subject: [PATCH 1/8] cover edge cases with respect to default power reduction in high precision networks --- app/app.go | 9 +++++++++ app/app_test.go | 5 +++++ x/rewards/keeper/min_cons_fee.go | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/app.go b/app/app.go index 3a7781e6..d6f642f6 100644 --- a/app/app.go +++ b/app/app.go @@ -3,6 +3,7 @@ package app import ( "fmt" "io" + "math/big" "net/http" "os" "path/filepath" @@ -228,6 +229,14 @@ var ( _ servertypes.Application = (*ArchwayApp)(nil) ) +const BaseDenomUnit = 18 + +func init() { + // sets the default power reduction in order to ensure that on high precision numbers, which is a default for archway + // the network does not get stalled due to an integer overflow in some edge cases. + sdk.DefaultPowerReduction = sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(BaseDenomUnit), nil)) // 10^18 +} + // ArchwayApp extended ABCI application type ArchwayApp struct { *baseapp.BaseApp diff --git a/app/app_test.go b/app/app_test.go index 8b846f16..e150cea1 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -2,6 +2,7 @@ package app import ( "encoding/json" + sdk "github.com/cosmos/cosmos-sdk/types" "os" "testing" @@ -17,6 +18,10 @@ import ( var emptyWasmOpts []wasm.Option = nil +func TestA(t *testing.T) { + t.Log(sdk.DefaultPowerReduction.String()) // 1_000_000_000_000_000_000 +} + func TestArchwaydExport(t *testing.T) { db := db.NewMemDB() gapp := NewArchwayApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeEncodingConfig(), wasm.EnableAllProposals, EmptyBaseAppOptions{}, emptyWasmOpts) diff --git a/x/rewards/keeper/min_cons_fee.go b/x/rewards/keeper/min_cons_fee.go index 7ebd707a..de62d6e6 100644 --- a/x/rewards/keeper/min_cons_fee.go +++ b/x/rewards/keeper/min_cons_fee.go @@ -61,7 +61,7 @@ func (k Keeper) ComputationalPriceOfGas(ctx sdk.Context) sdk.DecCoin { return minPoG } if minPoG.Denom != antiDoSPoG.Denom { - panic("conflict between anti dos denom and min price of gas denom") + panic("conflict between anti dos denom and min price of gas denom: %s != %s" + minPoG.Denom + antiDoSPoG.Denom) } return sdk.NewDecCoinFromDec(minPoG.Denom, sdk.MaxDec(minPoG.Amount, antiDoSPoG.Amount)) } From 8e0c7a46ca6034a971995a40398c355f4a0cb087 Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 05:48:53 +0200 Subject: [PATCH 2/8] add upgrade handler --- app/app_upgrades.go | 6 ++++-- app/upgrades/052/upgrades.go | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 app/upgrades/052/upgrades.go diff --git a/app/app_upgrades.go b/app/app_upgrades.go index 46236384..7f9a4fd2 100644 --- a/app/app_upgrades.go +++ b/app/app_upgrades.go @@ -8,13 +8,15 @@ import ( "github.com/archway-network/archway/app/upgrades" upgrade_0_3 "github.com/archway-network/archway/app/upgrades/03" upgrade_0_4 "github.com/archway-network/archway/app/upgrades/04" + upgrade_0_5_2 "github.com/archway-network/archway/app/upgrades/052" ) // UPGRADES var Upgrades = []upgrades.Upgrade{ - upgrade_0_3.Upgrade, // v0.3.0 - upgrade_0_4.Upgrade, // v0.4.0 + upgrade_0_3.Upgrade, // v0.3.0 + upgrade_0_4.Upgrade, // v0.4.0 + upgrade_0_5_2.Upgrade, // v0.5.2 } func (app *ArchwayApp) setupUpgrades() { diff --git a/app/upgrades/052/upgrades.go b/app/upgrades/052/upgrades.go new file mode 100644 index 00000000..31089aeb --- /dev/null +++ b/app/upgrades/052/upgrades.go @@ -0,0 +1,21 @@ +package upgrade052 + +import ( + "github.com/archway-network/archway/app/upgrades" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" +) + +const Name = "v0.5.2" + +var Upgrade = upgrades.Upgrade{ + UpgradeName: Name, + CreateUpgradeHandler: func(mm *module.Manager, cfg module.Configurator) upgradetypes.UpgradeHandler { + return func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + return mm.RunMigrations(ctx, cfg, fromVM) + } + }, + StoreUpgrades: storetypes.StoreUpgrades{}, +} From 8c99ed252bdf648b8eb3a9e351e0cb40aae76762 Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 06:16:47 +0200 Subject: [PATCH 3/8] fix tests --- app/app.go | 6 ++---- app/upgrades/052/precision_test.go | 12 ++++++++++++ e2e/gastracking_test.go | 7 ++++--- e2e/rewards_test.go | 4 ---- e2e/testing/chain_options.go | 7 ++++--- types/precision.go | 14 ++++++++++++++ 6 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 app/upgrades/052/precision_test.go create mode 100644 types/precision.go diff --git a/app/app.go b/app/app.go index d6f642f6..2080f905 100644 --- a/app/app.go +++ b/app/app.go @@ -3,7 +3,6 @@ package app import ( "fmt" "io" - "math/big" "net/http" "os" "path/filepath" @@ -112,6 +111,7 @@ import ( wasmclient "github.com/CosmWasm/wasmd/x/wasm/client" archwayappparams "github.com/archway-network/archway/app/params" + archway "github.com/archway-network/archway/types" // unnamed import of statik for swagger UI support _ "github.com/cosmos/cosmos-sdk/client/docs/statik" @@ -229,12 +229,10 @@ var ( _ servertypes.Application = (*ArchwayApp)(nil) ) -const BaseDenomUnit = 18 - func init() { // sets the default power reduction in order to ensure that on high precision numbers, which is a default for archway // the network does not get stalled due to an integer overflow in some edge cases. - sdk.DefaultPowerReduction = sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(BaseDenomUnit), nil)) // 10^18 + sdk.DefaultPowerReduction = archway.DefaultPowerReduction } // ArchwayApp extended ABCI application diff --git a/app/upgrades/052/precision_test.go b/app/upgrades/052/precision_test.go new file mode 100644 index 00000000..e1307be1 --- /dev/null +++ b/app/upgrades/052/precision_test.go @@ -0,0 +1,12 @@ +//go:build precision_test + +// NOTE: this test is run as a separate build tag as it modifies a global +// variable that is used in other tests. In a concurrent environment this +// might cause unforeseen issues, so we isolate it. +package upgrade052_test + +import "testing" + +func TestPrecisionBreakages(t *testing.T) { + +} diff --git a/e2e/gastracking_test.go b/e2e/gastracking_test.go index ca1fb5b5..9a60ce59 100644 --- a/e2e/gastracking_test.go +++ b/e2e/gastracking_test.go @@ -2,6 +2,7 @@ package e2e import ( "encoding/json" + archway "github.com/archway-network/archway/types" "strconv" "time" @@ -41,7 +42,7 @@ func (s *E2ETestSuite) TestGasTrackingAndRewardsDistribution() { 1000000, ), // Set default Tx fee for non-manual transaction like Upload / Instantiate - e2eTesting.WithDefaultFeeAmount("10000"), + e2eTesting.WithDefaultFeeAmount("500000000000000"), ) trackingKeeper, rewardsKeeper := chain.GetApp().TrackingKeeper, chain.GetApp().RewardsKeeper @@ -50,7 +51,7 @@ func (s *E2ETestSuite) TestGasTrackingAndRewardsDistribution() { accAddrs, accPrivKeys := e2eTesting.GenAccounts(1) // an empty account // Inputs - txFees := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000))) + txFees := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(500_000_000_000_000))) rewardsAcc := e2eTesting.Account{ Address: accAddrs[0], PrivKey: accPrivKeys[0], @@ -81,7 +82,7 @@ func (s *E2ETestSuite) TestGasTrackingAndRewardsDistribution() { } // Send some coins to the rewardsAcc to pay withdraw Tx fees - rewardsAccInitialBalance := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1000000))) + rewardsAccInitialBalance := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1).Mul(archway.DefaultPowerReduction))) { s.Require().NoError( chain.GetApp().BankKeeper.SendCoins( diff --git a/e2e/rewards_test.go b/e2e/rewards_test.go index 231d4e0e..ac6dc925 100644 --- a/e2e/rewards_test.go +++ b/e2e/rewards_test.go @@ -236,8 +236,6 @@ func (s *E2ETestSuite) TestRewardsWithdrawProfitAndFees() { // TestRewardsParamMaxWithdrawRecordsLimit check the x/rewards's MaxWithdrawRecords param limit (rough estimation). // Limit is defined by the block gas limit (100M). func (s *E2ETestSuite) TestRewardsParamMaxWithdrawRecordsLimit() { - s.T().Skip("Skipped to save CI time, should be used manually to estimate a new limit") - rewardsTypes.MaxWithdrawRecordsParamLimit = uint64(29500) // an actual value is (thisValue - 1), refer to the query below chain := e2eTesting.NewTestChain(s.T(), 1, @@ -312,8 +310,6 @@ func (s *E2ETestSuite) TestRewardsParamMaxWithdrawRecordsLimit() { // TestRewardsRecordsQueryLimit defines the x/rewards's RewardsRecords query limit (rough estimation). // Limit is defined by the max CosmWasm VM. func (s *E2ETestSuite) TestRewardsRecordsQueryLimit() { - s.T().Skip("Skipped to save CI time, should be used manually to estimate a new limit") - rewardsTypes.MaxRecordsQueryLimit = uint64(7716) // an actual value is (thisValue - 1), refer to the query below chain := e2eTesting.NewTestChain(s.T(), 1) diff --git a/e2e/testing/chain_options.go b/e2e/testing/chain_options.go index 25021772..c40fbb9b 100644 --- a/e2e/testing/chain_options.go +++ b/e2e/testing/chain_options.go @@ -1,6 +1,7 @@ package e2eTesting import ( + archway "github.com/archway-network/archway/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" mintTypes "github.com/cosmos/cosmos-sdk/x/mint/types" @@ -33,9 +34,9 @@ func defaultChainConfig() chainConfig { return chainConfig{ ValidatorsNum: 1, GenAccountsNum: 5, - GenBalanceAmount: "1000000000", - BondAmount: "1000000", - DefaultFeeAmt: "100", + GenBalanceAmount: archway.DefaultPowerReduction.MulRaw(100).String(), + BondAmount: archway.DefaultPowerReduction.MulRaw(1).String(), + DefaultFeeAmt: archway.DefaultPowerReduction.QuoRaw(10).String(), // 0.1 } } diff --git a/types/precision.go b/types/precision.go new file mode 100644 index 00000000..88ed83be --- /dev/null +++ b/types/precision.go @@ -0,0 +1,14 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "math/big" +) + +const ( + BaseDenomUnit = 18 +) + +var ( + DefaultPowerReduction = sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(BaseDenomUnit), nil)) // 10^18 +) From c9d3ac44e421867e8f8eb7fa52027d1822fe004b Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 06:51:03 +0200 Subject: [PATCH 4/8] create test vectors to assert fix --- app/upgrades/052/precision_test.go | 36 +++++++++++++++++++++++++++++- e2e/testing/chain_options.go | 6 +++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/app/upgrades/052/precision_test.go b/app/upgrades/052/precision_test.go index e1307be1..7eb58f2d 100644 --- a/app/upgrades/052/precision_test.go +++ b/app/upgrades/052/precision_test.go @@ -5,8 +5,42 @@ // might cause unforeseen issues, so we isolate it. package upgrade052_test -import "testing" +import ( + e2eTesting "github.com/archway-network/archway/e2e/testing" + archway "github.com/archway-network/archway/types" + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/stretchr/testify/require" + "testing" +) func TestPrecisionBreakages(t *testing.T) { + attoPrec := archway.DefaultPowerReduction + microPrecision := sdk.NewInt(1_000_000) + stake := func(shouldPass bool, errContains string) { + chainBad := e2eTesting.NewTestChain(t, 1, + e2eTesting.WithGenDefaultCoinBalance(attoPrec.MulRaw(1_000_000_000).String()), + e2eTesting.WithBondAmount(attoPrec.MulRaw(1).String()), + e2eTesting.WithDefaultFeeAmount(attoPrec.MulRaw(1).String()), + e2eTesting.WithValidatorsNum(2), + ) + acc := chainBad.GetAccount(1) + _, _, _, err := chainBad.SendMsgs(acc, shouldPass, []sdk.Msg{ + &stakingtypes.MsgDelegate{ + DelegatorAddress: acc.Address.String(), + ValidatorAddress: sdk.ValAddress(chainBad.GetCurrentValSet().Validators[0].Address).String(), + Amount: sdk.NewCoin("stake", attoPrec.MulRaw(100_000_000)), + }, + }) + if !shouldPass { + require.ErrorContains(t, err, errContains) + } + } + // setup bad power reduction + sdk.DefaultPowerReduction = microPrecision + stake(false, "Int64() out of bound") + // fix power reduction + sdk.DefaultPowerReduction = attoPrec + stake(true, "") } diff --git a/e2e/testing/chain_options.go b/e2e/testing/chain_options.go index c40fbb9b..5e66225f 100644 --- a/e2e/testing/chain_options.go +++ b/e2e/testing/chain_options.go @@ -40,6 +40,12 @@ func defaultChainConfig() chainConfig { } } +func WithValidatorsNum(num int) TestChainConfigOption { + return func(cfg *chainConfig) { + cfg.ValidatorsNum = num + } +} + // WithGenAccounts sets the number of genesis accounts func WithGenAccounts(num int) TestChainConfigOption { return func(cfg *chainConfig) { From 2585832ea0174a0530ca5e4ee0cfa8da5475982b Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 06:51:23 +0200 Subject: [PATCH 5/8] chore: lint --- app/app_test.go | 3 ++- app/upgrades/052/precision_test.go | 3 ++- e2e/gastracking_test.go | 3 ++- types/precision.go | 7 +++---- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index e150cea1..3033cc64 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -2,10 +2,11 @@ package app import ( "encoding/json" - sdk "github.com/cosmos/cosmos-sdk/types" "os" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/libs/log" diff --git a/app/upgrades/052/precision_test.go b/app/upgrades/052/precision_test.go index 7eb58f2d..b85e9a80 100644 --- a/app/upgrades/052/precision_test.go +++ b/app/upgrades/052/precision_test.go @@ -6,12 +6,13 @@ package upgrade052_test import ( + "testing" + e2eTesting "github.com/archway-network/archway/e2e/testing" archway "github.com/archway-network/archway/types" sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/stretchr/testify/require" - "testing" ) func TestPrecisionBreakages(t *testing.T) { diff --git a/e2e/gastracking_test.go b/e2e/gastracking_test.go index 9a60ce59..0029cfd7 100644 --- a/e2e/gastracking_test.go +++ b/e2e/gastracking_test.go @@ -2,10 +2,11 @@ package e2e import ( "encoding/json" - archway "github.com/archway-network/archway/types" "strconv" "time" + archway "github.com/archway-network/archway/types" + wasmdTypes "github.com/CosmWasm/wasmd/x/wasm/types" sdk "github.com/cosmos/cosmos-sdk/types" abci "github.com/tendermint/tendermint/abci/types" diff --git a/types/precision.go b/types/precision.go index 88ed83be..22d47fc6 100644 --- a/types/precision.go +++ b/types/precision.go @@ -1,14 +1,13 @@ package types import ( - sdk "github.com/cosmos/cosmos-sdk/types" "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" ) const ( BaseDenomUnit = 18 ) -var ( - DefaultPowerReduction = sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(BaseDenomUnit), nil)) // 10^18 -) +var DefaultPowerReduction = sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(BaseDenomUnit), nil)) // 10^18 From 8f304b8d31112f5e1ee80118f4cdbc182dee9482 Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 06:52:31 +0200 Subject: [PATCH 6/8] remove useless test --- app/app_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 3033cc64..8b846f16 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/libs/log" @@ -19,10 +17,6 @@ import ( var emptyWasmOpts []wasm.Option = nil -func TestA(t *testing.T) { - t.Log(sdk.DefaultPowerReduction.String()) // 1_000_000_000_000_000_000 -} - func TestArchwaydExport(t *testing.T) { db := db.NewMemDB() gapp := NewArchwayApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeEncodingConfig(), wasm.EnableAllProposals, EmptyBaseAppOptions{}, emptyWasmOpts) From bb3da35cff4286f10eb796c27dc63264a625e899 Mon Sep 17 00:00:00 2001 From: fdymylja Date: Sat, 13 May 2023 06:53:45 +0200 Subject: [PATCH 7/8] chore: CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c77d87f..2e88ce32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Contains all the PRs that improved the code without changing the behaviours. - [#368](https://github.com/archway-network/archway/pull/368) - github actions should fetch tags as well for deploy workflow - [#369](https://github.com/archway-network/archway/pull/369) - CODEOWNERS: small set to expand, not large set that filters - [#370](https://github.com/archway-network/archway/pull/370) - login to ghcr +- [#382](https://github.com/archway-network/archway/pull/382) - adjust default power reduction ### Changed From 3740b7fecaee7f24a1fcc8cb8d6b4ee6a518b724 Mon Sep 17 00:00:00 2001 From: Zanicar Date: Mon, 15 May 2023 08:55:14 +0200 Subject: [PATCH 8/8] chore: fix lint --- app/upgrades/052/upgrades.go | 3 ++- e2e/testing/chain_options.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/upgrades/052/upgrades.go b/app/upgrades/052/upgrades.go index 31089aeb..699bbca8 100644 --- a/app/upgrades/052/upgrades.go +++ b/app/upgrades/052/upgrades.go @@ -1,11 +1,12 @@ package upgrade052 import ( - "github.com/archway-network/archway/app/upgrades" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" + + "github.com/archway-network/archway/app/upgrades" ) const Name = "v0.5.2" diff --git a/e2e/testing/chain_options.go b/e2e/testing/chain_options.go index 5e66225f..4947656c 100644 --- a/e2e/testing/chain_options.go +++ b/e2e/testing/chain_options.go @@ -1,12 +1,13 @@ package e2eTesting import ( - archway "github.com/archway-network/archway/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" mintTypes "github.com/cosmos/cosmos-sdk/x/mint/types" abci "github.com/tendermint/tendermint/abci/types" + archway "github.com/archway-network/archway/types" + "github.com/archway-network/archway/app" rewardsTypes "github.com/archway-network/archway/x/rewards/types" )