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.

Sign up for GitHub

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

Revert "perf: reduce supported uptimes (#7651)" #7766

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Revert "perf: reduce supported uptimes (#7651)" #7766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#7619](https://github.com/osmosis-labs/osmosis/pull/7619) Slight speedup/gas improvement to CL GetTotalPoolLiquidity queries
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Create/remove tick events.
* [#7623](https://github.com/osmosis-labs/osmosis/pull/7623) Add query for querying all before send hooks
* [#7651](https://github.com/osmosis-labs/osmosis/pull/7651) Remove 1W and 2W supported uptimes for performance.
* [#7625](https://github.com/osmosis-labs/osmosis/pull/7625) Remove duplicate CL accumulator update logic.
* [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Remove duplicate CL accumulator update logic.
* [#7665](https://github.com/osmosis-labs/osmosis/pull/7665) feat(x/protorev): Use Transient store to store swap backruns.
* [#7685](https://github.com/osmosis-labs/osmosis/pull/7685) Speedup CL actions by only marshalling for CL hooks if they will be used.
* [#7503](https://github.com/osmosis-labs/osmosis/pull/7503) Add IBC wasm light clients module
Expand Down
14 changes: 13 additions & 1 deletion x/concentrated-liquidity/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,14 @@ var (
Options: nil,
}

// four records because we have 4 supported uptimes
// five records because we have 5 supported uptimes
testUptimeAccumRecord = []accum.Record{
accumRecord,
accumRecord,
accumRecord,
accumRecord,
accumRecord,
accumRecord,
}
)

Expand Down Expand Up @@ -236,6 +238,8 @@ func (s *KeeperTestSuite) TestInitGenesis() {
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(1000), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, hundredDec, osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, tenDec, osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(1), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(1), osmomath.NewInt(100), osmomath.NewInt(50)),
Comment on lines +241 to +242
Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of two identical accumRecordWithDefinedValues calls with the same parameters in the TestInitGenesis test case seems redundant. Given the context of increasing supported uptimes, it's likely that only one additional record was intended to be added. Please verify if this duplication is intentional or if one of these lines should be removed or modified to reflect a different test scenario.

},
},
},
Expand Down Expand Up @@ -296,6 +300,8 @@ func (s *KeeperTestSuite) TestInitGenesis() {
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(1000), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, hundredDec, osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, tenDec, osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(1), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(1), osmomath.NewInt(100), osmomath.NewInt(50)),
Comment on lines +303 to +304
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the previous comment, the addition of two identical accumRecordWithDefinedValues calls in the TestInitGenesis test case appears to be redundant. This duplication might be an oversight. Please confirm the intention behind these additions and consider removing or altering one of them if they were not meant to be identical.

},
},
},
Expand Down Expand Up @@ -381,6 +387,8 @@ func (s *KeeperTestSuite) TestInitGenesis() {
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9999), osmomath.NewInt(10), osmomath.NewInt(5)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(999), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(99), osmomath.NewInt(50), osmomath.NewInt(25)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9), osmomath.NewInt(50), osmomath.NewInt(25)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9), osmomath.NewInt(50), osmomath.NewInt(25)),
Comment on lines +390 to +391
Copy link
Contributor

Choose a reason for hiding this comment

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

In the TestInitGenesis test case, the addition of two identical accumRecordWithDefinedValues calls for the second pool scenario also seems redundant. This pattern of duplication is consistent with previous observations. Please review these additions to ensure they accurately represent the intended test scenarios and adjust as necessary.

},
},
},
Expand Down Expand Up @@ -472,6 +480,8 @@ func (s *KeeperTestSuite) TestInitGenesis() {
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9999), osmomath.NewInt(10), osmomath.NewInt(5)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(999), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(99), osmomath.NewInt(50), osmomath.NewInt(25)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9), osmomath.NewInt(50), osmomath.NewInt(25)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9), osmomath.NewInt(50), osmomath.NewInt(25)),
},
},
},
Expand Down Expand Up @@ -740,6 +750,8 @@ func (s *KeeperTestSuite) TestExportGenesis() {
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9999), osmomath.NewInt(10), osmomath.NewInt(5)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(999), osmomath.NewInt(100), osmomath.NewInt(50)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(99), osmomath.NewInt(50), osmomath.NewInt(25)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9), osmomath.NewInt(50), osmomath.NewInt(25)),
accumRecordWithDefinedValues(accumRecord, osmomath.NewDec(9), osmomath.NewInt(50), osmomath.NewInt(25)),
Comment on lines +753 to +754
Copy link
Contributor

Choose a reason for hiding this comment

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

The duplication of accumRecordWithDefinedValues calls in the TestExportGenesis test case for the second pool scenario mirrors the issues identified in the TestInitGenesis test case. This repetition may not be intentional and could potentially obscure the test's purpose. Please review and correct if necessary.

},
},
},
Expand Down
Loading