From 218af8d8d5154f227ac0fec67918edd6502bb577 Mon Sep 17 00:00:00 2001 From: Dzung Do | Decentrio Date: Thu, 6 Jun 2024 15:52:49 +0700 Subject: [PATCH] chore: Rework Denoms.Sort() to sort by base denom and then trace (#6493) * rework compare function to sort by base denom and add tests * add new test for Denoms.Sort() * fix TestMigratorMigrateDenomTraceToDenom with new Denoms.Sort() * fix typo * fix lint * fix logical bug * nit: rename test case and add test case --------- Co-authored-by: Carlos Rodriguez Co-authored-by: DimitrisJim --- .../apps/transfer/keeper/migrations_test.go | 2 +- modules/apps/transfer/types/denom.go | 12 +- modules/apps/transfer/types/denom_test.go | 149 ++++++++++++++++++ 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/keeper/migrations_test.go b/modules/apps/transfer/keeper/migrations_test.go index 3032f6344fe..141b4121ae8 100644 --- a/modules/apps/transfer/keeper/migrations_test.go +++ b/modules/apps/transfer/keeper/migrations_test.go @@ -119,8 +119,8 @@ func (suite *KeeperTestSuite) TestMigratorMigrateDenomTraceToDenom() { }, transfertypes.Denoms{ transfertypes.NewDenom("apple", transfertypes.NewTrace("transfer", "channel-0")), - transfertypes.NewDenom("pineapple", transfertypes.NewTrace("transfer", "channel-0")), transfertypes.NewDenom("cucumber", transfertypes.NewTrace("transfer", "channel-102"), transfertypes.NewTrace("transfer", "channel-0")), + transfertypes.NewDenom("pineapple", transfertypes.NewTrace("transfer", "channel-0")), transfertypes.NewDenom("uatom", transfertypes.NewTrace("transfer", "channel-49")), }, }, diff --git a/modules/apps/transfer/types/denom.go b/modules/apps/transfer/types/denom.go index 995a7e81898..f6a8a6abdfb 100644 --- a/modules/apps/transfer/types/denom.go +++ b/modules/apps/transfer/types/denom.go @@ -122,7 +122,17 @@ var _ sort.Interface = (*Denoms)(nil) func (d Denoms) Len() int { return len(d) } // Less implements sort.Interface for Denoms -func (d Denoms) Less(i, j int) bool { return d[i].Path() < d[j].Path() } +func (d Denoms) Less(i, j int) bool { + if d[i].Base != d[j].Base { + return d[i].Base < d[j].Base + } + + if len(d[i].Trace) != len(d[j].Trace) { + return len(d[i].Trace) < len(d[j].Trace) + } + + return d[i].Path() < d[j].Path() +} // Swap implements sort.Interface for Denoms func (d Denoms) Swap(i, j int) { d[i], d[j] = d[j], d[i] } diff --git a/modules/apps/transfer/types/denom_test.go b/modules/apps/transfer/types/denom_test.go index 08919c4a3f3..659f31d7607 100644 --- a/modules/apps/transfer/types/denom_test.go +++ b/modules/apps/transfer/types/denom_test.go @@ -119,6 +119,155 @@ func (suite *TypesTestSuite) TestPath() { } } +func (suite *TypesTestSuite) TestSort() { + testCases := []struct { + name string + denoms types.Denoms + expDenoms types.Denoms + }{ + { + "only base denom", + types.Denoms{types.Denom{Base: "uosmo"}, types.Denom{Base: "gamm"}, types.Denom{Base: "uatom"}}, + types.Denoms{types.Denom{Base: "gamm"}, types.Denom{Base: "uatom"}, types.Denom{Base: "uosmo"}}, + }, + { + "different base denom and same traces", + types.Denoms{ + types.Denom{ + Base: "uosmo", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + }, + types.Denoms{ + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "uosmo", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + }, + }, + { + "same base denom and different traces", + types.Denoms{ + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("mountain", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "uatom", + }, + }, + types.Denoms{ + types.Denom{ + Base: "uatom", + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("mountain", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-52")}, + }, + }, + }, + { + "different base denoms and different traces", + types.Denoms{ + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("pool", "channel-0")}, + }, + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("pool", "channel-0"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "utia", + }, + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52")}, + }, + }, + types.Denoms{ + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("pool", "channel-0")}, + }, + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("pool", "channel-0"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "gamm", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0")}, + }, + types.Denom{ + Base: "uatom", + Trace: []types.Trace{types.NewTrace("transfer", "channel-0"), types.NewTrace("transfer", "channel-52"), types.NewTrace("transfer", "channel-52")}, + }, + types.Denom{ + Base: "utia", + }, + }, + }, + } + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.Require().Equal(tc.expDenoms, tc.denoms.Sort()) + }) + } +} + func (suite *TypesTestSuite) TestDenomChainSource() { testCases := []struct { name string