diff --git a/CHANGELOG.md b/CHANGELOG.md index 361c850ea172..993ed6798942 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting. * [\#9762](https://github.com/cosmos/cosmos-sdk/pull/9762) The init command uses the chain-id from the client config if --chain-id is not provided +### State Machine Breaking + +* (x/bank) [\#9611](https://github.com/cosmos/cosmos-sdk/pull/9611) Introduce a new index to act as a reverse index between a denomination and address allowing to query for + token holders of a specific denomination. `DenomOwners` is updated to use the new reverse index. + ## [v0.43.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc0) - 2021-06-25 ### Features diff --git a/container/option_test.go b/container/option_test.go new file mode 100644 index 000000000000..f447afa5a964 --- /dev/null +++ b/container/option_test.go @@ -0,0 +1 @@ +package container_test diff --git a/docs/architecture/adr-043-nft-module.md b/docs/architecture/adr-043-nft-module.md index 814be6cb1898..8c2f967b1694 100644 --- a/docs/architecture/adr-043-nft-module.md +++ b/docs/architecture/adr-043-nft-module.md @@ -281,6 +281,15 @@ message QueryClassesResponse { } ``` + +### Interoperability + +Interoperability is all about reusing assets between modules and chains. The former one is achieved by ADR-33: Protobuf client - server communication. At the time of writing ADR-33 is not finalized. The latter is achieved by IBC. Here we will focus on the IBC side. +IBC is implemented per module. Here, we aligned that NFTs will be recorded and managed in the x/nft. This requires creation of a new IBC standard and implementation of it. + +For IBC interoperability, NFT custom modules MUST use the NFT object type understood by the IBC client. So, for x/nft interoperability, custom NFT implementations (example: x/cryptokitty) should use the canonical x/nft module and proxy all NFT balance keeping functionality to x/nft or else re-implement all functionality using the NFT object type understood by the IBC client. In other words: x/nft becomes the standard NFT registry for all Cosmos NFTs (example: x/cryptokitty will register a kitty NFT in x/nft and use x/nft for book keeping). This was [discussed](https://github.com/cosmos/cosmos-sdk/discussions/9065#discussioncomment-873206) in the context of using x/bank as a general asset balance book. Not using x/nft will require implementing another module for IBC. + + ## Consequences ### Backward Compatibility @@ -299,6 +308,7 @@ This specification conforms to the ERC-721 smart contract specification for NFT ### Negative ++ New IBC app is required for x/nft ### Neutral diff --git a/simapp/app_test.go b/simapp/app_test.go index c84c99003053..76c1a423d1f9 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -123,8 +123,13 @@ func TestRunMigrations(t *testing.T) { false, "", true, "no migrations found for module bank: not found", 0, }, { - "can register and run migration handler for x/bank", + "can register 1->2 migration handler for x/bank, cannot run migration", "bank", 1, + false, "", true, "no migration found for module bank from version 2 to version 3: not found", 0, + }, + { + "can register 2->3 migration handler for x/bank, can run migration", + "bank", 2, false, "", false, "", 1, }, { diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go index 938f7dddffb4..c0cf44cd60d4 100644 --- a/types/dec_coin_test.go +++ b/types/dec_coin_test.go @@ -112,6 +112,7 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { input sdk.DecCoins original string expected string + panic bool }{ { name: "all greater than zero", @@ -124,6 +125,7 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { }, original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + panic: false, }, { name: "zero coin in middle", @@ -136,6 +138,7 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { }, original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste", + panic: false, }, { name: "zero coin end (unordered)", @@ -148,13 +151,32 @@ func (s *decCoinTestSuite) TestFilteredZeroDecCoins() { }, original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb", expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + panic: false, + }, + + { + name: "panic when same denoms in multiple coins", + input: sdk.DecCoins{ + {"testa", sdk.NewDec(5)}, + {"testa", sdk.NewDec(3)}, + {"testa", sdk.NewDec(1)}, + {"testd", sdk.NewDec(4)}, + {"testb", sdk.NewDec(2)}, + }, + original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb", + expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste", + panic: true, }, } for _, tt := range cases { - undertest := sdk.NewDecCoins(tt.input...) - s.Require().Equal(tt.expected, undertest.String(), "NewDecCoins must return expected results") - s.Require().Equal(tt.original, tt.input.String(), "input must be unmodified and match original") + if tt.panic { + s.Require().Panics(func() { sdk.NewDecCoins(tt.input...) }, "Should panic due to multiple coins with same denom") + } else { + undertest := sdk.NewDecCoins(tt.input...) + s.Require().Equal(tt.expected, undertest.String(), "NewDecCoins must return expected results") + s.Require().Equal(tt.original, tt.input.String(), "input must be unmodified and match original") + } } } @@ -580,3 +602,543 @@ func (s *decCoinTestSuite) TestDecCoins_AddDecCoinWithIsValid() { } } } + +func (s *decCoinTestSuite) TestDecCoins_Empty() { + testCases := []struct { + input sdk.DecCoins + expectedResult bool + msg string + }{ + {sdk.DecCoins{}, true, "No coins as expected."}, + {sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, false, "DecCoins is not empty"}, + } + + for _, tc := range testCases { + if tc.expectedResult { + s.Require().True(tc.input.Empty(), tc.msg) + } else { + s.Require().False(tc.input.Empty(), tc.msg) + } + } +} + +func (s *decCoinTestSuite) TestDecCoins_GetDenomByIndex() { + testCases := []struct { + name string + input sdk.DecCoins + index int + expectedResult string + expectedErr bool + }{ + { + "No DecCoins in Slice", + sdk.DecCoins{}, + 0, + "", + true, + }, + {"When index out of bounds", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, 2, "", true}, + {"When negative index", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, -1, "", true}, + { + "Appropriate index case", + sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(5)}, + sdk.DecCoin{testDenom2, sdk.NewDec(57)}, + }, 1, testDenom2, false, + }, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedErr { + s.Require().Panics(func() { tc.input.GetDenomByIndex(tc.index) }, "Test should have panicked") + } else { + res := tc.input.GetDenomByIndex(tc.index) + s.Require().Equal(tc.expectedResult, res, "Unexpected result for test case #%d, expected output: %s, input: %v", i, tc.expectedResult, tc.input) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_IsAllPositive() { + testCases := []struct { + name string + input sdk.DecCoins + expectedResult bool + }{ + {"No Coins", sdk.DecCoins{}, false}, + + {"One Coin - Zero value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(0)}}, false}, + + {"One Coin - Postive value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, true}, + + {"One Coin - Negative value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(-15)}}, false}, + + {"Multiple Coins - All positive value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(51)}, + sdk.DecCoin{testDenom1, sdk.NewDec(123)}, + sdk.DecCoin{testDenom1, sdk.NewDec(50)}, + sdk.DecCoin{testDenom1, sdk.NewDec(92233720)}, + }, true}, + + {"Multiple Coins - Some negative value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(51)}, + sdk.DecCoin{testDenom1, sdk.NewDec(-123)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(92233720)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedResult { + s.Require().True(tc.input.IsAllPositive(), "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(tc.input.IsAllPositive(), "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_IsLT() { + testCases := []struct { + name string + coin sdk.DecCoin + otherCoin sdk.DecCoin + expectedResult bool + expectedPanic bool + }{ + + {"Same Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom1, sdk.NewDec(19)}, true, false}, + + {"Same Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom1, sdk.NewDec(14)}, false, false}, + + {"Same Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom1, sdk.NewDec(20)}, false, false}, + + {"Different Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom2, sdk.NewDec(19)}, true, true}, + + {"Different Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom2, sdk.NewDec(14)}, true, true}, + + {"Different Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom2, sdk.NewDec(20)}, true, true}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coin.IsLT(tc.otherCoin) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coin.IsLT(tc.otherCoin) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_IsGTE() { + testCases := []struct { + name string + coin sdk.DecCoin + otherCoin sdk.DecCoin + expectedResult bool + expectedPanic bool + }{ + + {"Same Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom1, sdk.NewDec(19)}, false, false}, + + {"Same Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom1, sdk.NewDec(14)}, true, false}, + + {"Same Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom1, sdk.NewDec(20)}, true, false}, + + {"Different Denom - Less than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(3)}, sdk.DecCoin{testDenom2, sdk.NewDec(19)}, true, true}, + + {"Different Denom - Greater than other coin", sdk.DecCoin{testDenom1, sdk.NewDec(343340)}, sdk.DecCoin{testDenom2, sdk.NewDec(14)}, true, true}, + + {"Different Denom - Same as other coin", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, sdk.DecCoin{testDenom2, sdk.NewDec(20)}, true, true}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coin.IsGTE(tc.otherCoin) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coin.IsGTE(tc.otherCoin) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_IsZero() { + testCases := []struct { + name string + coins sdk.DecCoins + expectedResult bool + }{ + {"No Coins", sdk.DecCoins{}, true}, + + {"One Coin - Zero value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(0)}}, true}, + + {"One Coin - Postive value", sdk.DecCoins{sdk.DecCoin{testDenom1, sdk.NewDec(5)}}, false}, + + {"Multiple Coins - All zero value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + }, true}, + + {"Multiple Coins - Some positive value", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(0)}, + sdk.DecCoin{testDenom1, sdk.NewDec(92233720)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedResult { + s.Require().True(tc.coins.IsZero(), "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(tc.coins.IsZero(), "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_MulDec() { + testCases := []struct { + name string + coins sdk.DecCoins + multiplier sdk.Dec + expectedResult sdk.DecCoins + }{ + {"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil)}, + + {"Multiple coins - zero multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(10)}, + sdk.DecCoin{testDenom1, sdk.NewDec(30)}, + }, sdk.NewDec(0), sdk.DecCoins(nil)}, + + {"Multiple coins - positive multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom1, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + }}, + + {"Multiple coins - negative multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom1, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(-2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(-20)}, + }}, + + {"Multiple coins - Different denom", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom2, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom2, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(8)}, + sdk.DecCoin{testDenom2, sdk.NewDec(12)}, + }}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + res := tc.coins.MulDec(tc.multiplier) + s.Require().Equal(tc.expectedResult, res, "Test case #%d: %s", i, tc.name) + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_MulDecTruncate() { + testCases := []struct { + name string + coins sdk.DecCoins + multiplier sdk.Dec + expectedResult sdk.DecCoins + expectedPanic bool + }{ + {"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil), false}, + + // TODO - Fix test - Function comment documentation for MulDecTruncate says if multiplier d is zero, it should panic. + // However, that is not the observed behaviour. Currently nil is returned. + {"Multiple coins - zero multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(10, 3)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(30, 2)}, + }, sdk.NewDec(0), sdk.DecCoins(nil), false}, + + {"Multiple coins - positive multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + }, sdk.NewDec(1), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(3, 0)}, + }, false}, + + {"Multiple coins - positive multiplier", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + }, sdk.NewDec(-2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(-6, 0)}, + }, false}, + + {"Multiple coins - Different denom", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom2, sdk.NewDecWithPrec(3333, 4)}, + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(15, 1)}, + sdk.DecCoin{testDenom2, sdk.NewDecWithPrec(333, 4)}, + }, sdk.NewDec(10), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(30, 0)}, + sdk.DecCoin{testDenom2, sdk.NewDecWithPrec(3666, 3)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coins.MulDecTruncate(tc.multiplier) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coins.MulDecTruncate(tc.multiplier) + s.Require().Equal(tc.expectedResult, res, "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_QuoDec() { + + testCases := []struct { + name string + coins sdk.DecCoins + input sdk.Dec + expectedResult sdk.DecCoins + panics bool + }{ + {"No Coins", sdk.DecCoins{}, sdk.NewDec(1), sdk.DecCoins(nil), false}, + + {"Multiple coins - zero input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(10)}, + sdk.DecCoin{testDenom1, sdk.NewDec(30)}, + }, sdk.NewDec(0), sdk.DecCoins(nil), true}, + + {"Multiple coins - positive input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(35, 1)}, + }, false}, + + {"Multiple coins - negative input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.NewDec(-2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDecWithPrec(-35, 1)}, + }, false}, + + {"Multiple coins - Different input", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(1)}, + sdk.DecCoin{testDenom2, sdk.NewDec(2)}, + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom2, sdk.NewDec(4)}, + }, sdk.NewDec(2), sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(2)}, + sdk.DecCoin{testDenom2, sdk.NewDec(3)}, + }, false}, + } + + for i, tc := range testCases { + tc := tc + s.T().Run(tc.name, func(t *testing.T) { + if tc.panics { + s.Require().Panics(func() { tc.coins.QuoDec(tc.input) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coins.QuoDec(tc.input) + s.Require().Equal(tc.expectedResult, res, "Test case #%d: %s", i, tc.name) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_IsEqual() { + testCases := []struct { + name string + coin sdk.DecCoin + otherCoin sdk.DecCoin + expectedResult bool + expectedPanic bool + }{ + + {"Different Denom Same Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom2, sdk.NewDec(20)}, + false, true}, + + {"Different Denom Different Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom2, sdk.NewDec(10)}, + false, true}, + + {"Same Denom Different Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom1, sdk.NewDec(10)}, + false, false}, + + {"Same Denom Same Amount", sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + sdk.DecCoin{testDenom1, sdk.NewDec(20)}, + true, false}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coin.IsEqual(tc.otherCoin) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coin.IsEqual(tc.otherCoin) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoins_IsEqual() { + testCases := []struct { + name string + coinsA sdk.DecCoins + coinsB sdk.DecCoins + expectedResult bool + expectedPanic bool + }{ + {"Different length sets", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(35)}, + }, false, false}, + + {"Same length - different denoms", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom2, sdk.NewDec(3)}, + sdk.DecCoin{testDenom2, sdk.NewDec(4)}, + }, false, true}, + + {"Same length - different amounts", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(3)}, + sdk.DecCoin{testDenom1, sdk.NewDec(4)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(41)}, + sdk.DecCoin{testDenom1, sdk.NewDec(343)}, + }, false, false}, + + {"Same length - same amounts", sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(33)}, + sdk.DecCoin{testDenom1, sdk.NewDec(344)}, + }, sdk.DecCoins{ + sdk.DecCoin{testDenom1, sdk.NewDec(33)}, + sdk.DecCoin{testDenom1, sdk.NewDec(344)}, + }, true, false}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + if tc.expectedPanic { + s.Require().Panics(func() { tc.coinsA.IsEqual(tc.coinsB) }, "Test case #%d: %s", i, tc.name) + } else { + res := tc.coinsA.IsEqual(tc.coinsB) + if tc.expectedResult { + s.Require().True(res, "Test case #%d: %s", i, tc.name) + } else { + s.Require().False(res, "Test case #%d: %s", i, tc.name) + } + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_Validate() { + var empty sdk.DecCoin + testCases := []struct { + name string + input sdk.DecCoin + expectedPass bool + }{ + {"Uninitalized deccoin", empty, false}, + + {"Invalid denom string", sdk.DecCoin{"(){9**&})", sdk.NewDec(33)}, false}, + + {"Negative coin amount", sdk.DecCoin{testDenom1, sdk.NewDec(-33)}, false}, + + {"Valid coin", sdk.DecCoin{testDenom1, sdk.NewDec(33)}, true}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + err := tc.input.Validate() + if tc.expectedPass { + s.Require().NoError(err, "unexpected result for test case #%d %s, input: %v", i, tc.name, tc.input) + } else { + s.Require().Error(err, "unexpected result for test case #%d %s, input: %v", i, tc.name, tc.input) + } + }) + } +} + +func (s *decCoinTestSuite) TestDecCoin_ParseDecCoin() { + var empty sdk.DecCoin + testCases := []struct { + name string + input string + expectedResult sdk.DecCoin + expectedErr bool + }{ + {"Empty input", "", empty, true}, + + {"Bad input", "✨🌟⭐", empty, true}, + + {"Invalid decimal coin", "9.3.0stake", empty, true}, + + {"Precision over limit", "9.11111111111111111111stake", empty, true}, + + // TODO - Clarify - According to error message for ValidateDenom call, supposed to + // throw error when upper case characters are used. Currently uppercase denoms are allowed. + {"Invalid denom", "9.3STAKE", sdk.DecCoin{"STAKE", sdk.NewDecWithPrec(93, 1)}, false}, + + {"Valid input - amount and denom seperated by space", "9.3 stake", sdk.DecCoin{"stake", sdk.NewDecWithPrec(93, 1)}, false}, + + {"Valid input - amount and denom concatenated", "9.3stake", sdk.DecCoin{"stake", sdk.NewDecWithPrec(93, 1)}, false}, + } + + for i, tc := range testCases { + s.T().Run(tc.name, func(t *testing.T) { + res, err := sdk.ParseDecCoin(tc.input) + if tc.expectedErr { + s.Require().Error(err, "expected error for test case #%d %s, input: %v", i, tc.name, tc.input) + } else { + s.Require().NoError(err, "unexpected error for test case #%d %s, input: %v", i, tc.name, tc.input) + s.Require().Equal(tc.expectedResult, res, "unexpected result for test case #%d %s, input: %v", i, tc.name, tc.input) + } + }) + } +} diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index dc843db9fac6..06e47e0164c3 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -1045,12 +1045,11 @@ func (s *IntegrationTestSuite) TestTxWithoutPublicKey() { s.Require().NotEqual(0, res.Code) } +// TestSignWithMultiSignersAminoJSON tests the case where a transaction with 2 +// messages which has to be signed with 2 different keys. Sign and append the +// signatures using the CLI with Amino signing mode. Finally, send the +// transaction to the blockchain. func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() { - // test case: - // Create a transaction with 2 messages which has to be signed with 2 different keys - // Sign and append the signatures using the CLI with Amino signing mode. - // Finally send the transaction to the blockchain. It should work. - require := s.Require() val0, val1 := s.network.Validators[0], s.network.Validators[1] val0Coin := sdk.NewCoin(fmt.Sprintf("%stoken", val0.Moniker), sdk.NewInt(10)) @@ -1067,7 +1066,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() { banktypes.NewMsgSend(val1.Address, addr1, sdk.NewCoins(val1Coin)), ) txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)))) - txBuilder.SetGasLimit(testdata.NewTestGasLimit()) // min required is 101892 + txBuilder.SetGasLimit(testdata.NewTestGasLimit() * 2) require.Equal([]sdk.AccAddress{val0.Address, val1.Address}, txBuilder.GetTx().GetSigners()) // Write the unsigned tx into a file. @@ -1083,14 +1082,19 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() { // Then let val1 sign the file with signedByVal0. val1AccNum, val1Seq, err := val0.ClientCtx.AccountRetriever.GetAccountNumberSequence(val0.ClientCtx, val1.Address) require.NoError(err) + signedTx, err := TxSignExec( - val1.ClientCtx, val1.Address, signedByVal0File.Name(), - "--offline", fmt.Sprintf("--account-number=%d", val1AccNum), fmt.Sprintf("--sequence=%d", val1Seq), "--sign-mode=amino-json", + val1.ClientCtx, + val1.Address, + signedByVal0File.Name(), + "--offline", + fmt.Sprintf("--account-number=%d", val1AccNum), + fmt.Sprintf("--sequence=%d", val1Seq), + "--sign-mode=amino-json", ) require.NoError(err) signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String()) - // Now let's try to send this tx. res, err := TxBroadcastExec( val0.ClientCtx, signedTxFile.Name(), @@ -1100,7 +1104,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() { require.NoError(err) var txRes sdk.TxResponse require.NoError(val0.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes)) - require.Equal(uint32(0), txRes.Code) + require.Equal(uint32(0), txRes.Code, txRes.RawLog) // Make sure the addr1's balance got funded. queryResJSON, err := bankcli.QueryBalancesExec(val0.ClientCtx, addr1) diff --git a/x/auth/tx/decoder.go b/x/auth/tx/decoder.go index d93c7446db60..2fef6312b994 100644 --- a/x/auth/tx/decoder.go +++ b/x/auth/tx/decoder.go @@ -16,7 +16,7 @@ import ( func DefaultTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { return func(txBytes []byte) (sdk.Tx, error) { // Make sure txBytes follow ADR-027. - err := rejectNonADR027(txBytes) + err := rejectNonADR027TxRaw(txBytes) if err != nil { return nil, sdkerrors.Wrap(sdkerrors.ErrTxDecode, err.Error()) } @@ -90,13 +90,14 @@ func DefaultJSONTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder { } } -// rejectNonADR027 rejects txBytes that do not follow ADR-027. This function +// rejectNonADR027TxRaw rejects txBytes that do not follow ADR-027. This is NOT +// a generic ADR-027 checker, it only applies decoding TxRaw. Specifically, it // only checks that: // - field numbers are in ascending order (1, 2, and potentially multiple 3s), -// - and varints as as short as possible. -// All other ADR-027 edge cases (e.g. TxRaw fields having default values) will -// not happen with TxRaw. -func rejectNonADR027(txBytes []byte) error { +// - and varints are as short as possible. +// All other ADR-027 edge cases (e.g. default values) are not applicable with +// TxRaw. +func rejectNonADR027TxRaw(txBytes []byte) error { // Make sure all fields are ordered in ascending order with this variable. prevTagNum := protowire.Number(0) @@ -105,21 +106,25 @@ func rejectNonADR027(txBytes []byte) error { if m < 0 { return fmt.Errorf("invalid length; %w", protowire.ParseError(m)) } + // TxRaw only has bytes fields. if wireType != protowire.BytesType { - return fmt.Errorf("expected %d wire type, got %d", protowire.VarintType, wireType) + return fmt.Errorf("expected %d wire type, got %d", protowire.BytesType, wireType) } + // Make sure fields are ordered in ascending order. if tagNum < prevTagNum { return fmt.Errorf("txRaw must follow ADR-027, got tagNum %d after tagNum %d", tagNum, prevTagNum) } prevTagNum = tagNum // All 3 fields of TxRaw have wireType == 2, so their next component - // is a varint. - // We make sure that the varint is as short as possible. + // is a varint, so we can safely call ConsumeVarint here. + // Byte structure: + // Inner fields are verified in `DefaultTxDecoder` lengthPrefix, m := protowire.ConsumeVarint(txBytes[m:]) if m < 0 { return fmt.Errorf("invalid length; %w", protowire.ParseError(m)) } + // We make sure that this varint is as short as possible. n := varintMinLength(lengthPrefix) if n != m { return fmt.Errorf("length prefix varint for tagNum %d is not as short as possible, read %d, only need %d", tagNum, m, n) @@ -141,23 +146,23 @@ func rejectNonADR027(txBytes []byte) error { func varintMinLength(n uint64) int { switch { // Note: 1< byte(amount)` -- Denom Metadata: `0x1 | byte(denom) -> ProtocolBuffer(Metadata)` -- Balances: `0x2 | byte(address length) | []byte(address) | []byte(balance.Denom) -> ProtocolBuffer(balance)` +1. Account balances +2. Denomination metadata +3. The total supply of all balances + +In addition, the `x/bank` module keeps the following indexes to manage the +aforementioned state: + +- Supply Index: `0x0 | byte(denom) -> byte(amount)` +- Denom Metadata Index: `0x1 | byte(denom) -> ProtocolBuffer(Metadata)` +- Balances Index: `0x2 | byte(address length) | []byte(address) | []byte(balance.Denom) -> ProtocolBuffer(balance)` +- Reverse Denomination to Address Index: `0x03 | byte(denom) | 0x00 | []byte(address) -> 0` diff --git a/x/bank/types/key.go b/x/bank/types/key.go index e91a38970e8b..be8f43f9aebe 100644 --- a/x/bank/types/key.go +++ b/x/bank/types/key.go @@ -22,11 +22,13 @@ const ( // KVStore keys var ( - // BalancesPrefix is the prefix for the account balances store. We use a byte - // (instead of `[]byte("balances")` to save some disk space). - BalancesPrefix = []byte{0x02} SupplyKey = []byte{0x00} DenomMetadataPrefix = []byte{0x1} + DenomAddressPrefix = []byte{0x03} + + // BalancesPrefix is the prefix for the account balances store. We use a byte + // (instead of `[]byte("balances")` to save some disk space). + BalancesPrefix = []byte{0x02} ) // DenomMetadataKey returns the denomination metadata key. @@ -44,12 +46,16 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) { if len(key) == 0 { return nil, ErrInvalidKey } + kv.AssertKeyAtLeastLength(key, 1) + addrLen := key[0] bound := int(addrLen) + if len(key)-1 < bound { return nil, ErrInvalidKey } + return key[1 : bound+1], nil } @@ -57,3 +63,10 @@ func AddressFromBalancesStore(key []byte) (sdk.AccAddress, error) { func CreateAccountBalancesPrefix(addr []byte) []byte { return append(BalancesPrefix, address.MustLengthPrefix(addr)...) } + +// CreateDenomAddressPrefix creates a prefix for a reverse index of denomination +// to account balance for that denomination. +func CreateDenomAddressPrefix(denom string) []byte { + key := append(DenomAddressPrefix, []byte(denom)...) + return append(key, 0) +} diff --git a/x/staking/client/testutil/suite.go b/x/staking/client/testutil/suite.go index 965c35823c57..4cbae20eba43 100644 --- a/x/staking/client/testutil/suite.go +++ b/x/staking/client/testutil/suite.go @@ -57,15 +57,18 @@ func (s *IntegrationTestSuite) SetupSuite() { val2 := s.network.Validators[1] // redelegate - _, err = MsgRedelegateExec( + out, err := MsgRedelegateExec( val.ClientCtx, val.Address, val.ValAddress, val2.ValAddress, unbond, - fmt.Sprintf("--%s=%d", flags.FlagGas, 202954), // 202954 is the required + fmt.Sprintf("--%s=%d", flags.FlagGas, 300000), ) s.Require().NoError(err) + var txRes sdk.TxResponse + s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes)) + s.Require().Equal(uint32(0), txRes.Code) _, err = s.network.WaitForHeight(1) s.Require().NoError(err) // unbonding @@ -1181,7 +1184,7 @@ func (s *IntegrationTestSuite) TestNewRedelegateCmd() { val2.ValAddress.String(), // dst-validator-addr sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(150)).String(), // amount fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()), - fmt.Sprintf("--%s=%s", flags.FlagGas, "auto"), + fmt.Sprintf("--%s=%d", flags.FlagGas, 300000), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),