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

chore: fixing linter errors and tests #463

Merged
merged 15 commits into from
Oct 4, 2023

Conversation

spoo-bar
Copy link
Contributor

@spoo-bar spoo-bar commented Sep 27, 2023

  • Fixing lint issues
  • Added ibctm.AppModuleBasic{} to app wiring as it was missing
  • Fixing tests by updating the APIs wherever necessary, such as
    • Replacing sdk.Int with math.Int
    • Replacing gov prop msgs from v1beta1 type to v1 type
    • Replacing txRes.Data references to txRes.MsgResponses as Data field is deprecated now
    • Using interchaintest new genesis cmd cuz now we sdk47
    • Removing the custom interchaintest fork go mod replace as now the framework accepts custom genesis amounts
  • Removing ante.NewDeductFeeDecorator which was added in chore: updating app wiring #453 Turns out the upgrade instructions of sdk 0.46x which said to replace MempoolFeeDecorator with DeductFeeDecorator is a bit of a problem. MempoolFeeDecorator used to check if the txfees met the requirements. DeductFeeDecorator does that PLUS deducts the fees too. Not directly substitutable now is it. So this new ante is now removed as we have a custom deduct fee ante handler anyway.
  • E2E tests fixed by increasing setup numbers by 1000x. Due to high DefaultPowerReduction, older values were not enough to start the chain and sdk would panic. (This issue should have been caught when precision was modified. but SDK didnt complaim then, so we didnt know. Thanks @fdymylja for the help with this)

Note

One test TestMintBankKeeper/OK:_100stake:_Mint_->_FeeCollector_with_0.01_ratio_to_Rewards_(no_block_gas_limit) is still failing. But that is kinda to be expected because of an upstream change in sdk changing how GasMeter limit are handled when BlockGas Limit is -1. Would like to discuss with @fdymylja and @zanicar and fix this in a new PR

I would like to end this PR by thanking the previous devs of this repo 🙏🏻 who have written such extensive tests that we caught so many gotcha scenarios. The tests fixing was a pain but it was a pain cuz it had such comprehensive coverage.

@spoo-bar spoo-bar changed the title chore: fixing linter errors chore: fixing linter errors and tests Oct 3, 2023
@spoo-bar spoo-bar marked this pull request as ready for review October 4, 2023 12:40
@spoo-bar spoo-bar requested a review from a team as a code owner October 4, 2023 12:40
@spoo-bar spoo-bar requested review from zanicar and removed request for a team October 4, 2023 12:40
Copy link
Contributor

@zanicar zanicar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@fdymylja fdymylja left a comment

Choose a reason for hiding this comment

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

LGTM.

@spoo-bar spoo-bar merged commit d5c0374 into feat/sdk-47-upgrade Oct 4, 2023
4 of 5 checks passed
@spoo-bar spoo-bar deleted the spoorthi/fix-lint-and-tests branch October 4, 2023 15:11
spoo-bar added a commit that referenced this pull request Oct 13, 2023
* chore: replacing tendermint with cometbft (#433)

* replacing tendermint refs with comet bft

* replacing tm-db with cometbft-db

* chore: Bumping sdk, wasmd and ibc-go (#435)

* bumping sdk and wasmd and ibc versions

* bumping ibc-go refs

* chore: protobuf migration (#436)

* protobuf migration

* removing third_party/

* regenerating the proto files

* RecordIDs.Ids is a repeated non-nullable native type, nullable=false has no effect

* chore: fix broken upstream references (#450)

updating upstream references for x/wasm. ibc-go, cosmos-sdk

* chore: remove deprecated appmodule routes (#451)

* remove genmsg deprecated route

* remove rewards module deprecated route

* remove tracking module deprecated route

* chore: fixing all the modules and wasmbindings   (#452)

* fixing x/tracking module

* fixing x/rewards module

* fixing x/genmsg module

* fixing genmsg module.go

* fixing wasmbinding

* chore: updating app wiring (#453)

* fixing the upgrade handlers

* fixing ante.go

* fixing export.go

* updating sim_test

* fixing test_helpers

* fixing some app.go stuff

* custom "add-genesis-account" - removed as its not needed anymore

* fixing root.go

* fixing simulation test

* fixing app.go

* cleanup

* chore: fixing e2e tests (#454)

* fixing some e2e tests

* fix more e2e tests

* fixing ibc e2e stuff

* updating changes from merge

* fixing golang.org/x/exp ref. thx @fdymylja

Co-Authored-By: Frojdi Dymylja <33157909+fdymylja@users.noreply.github.com>

* pr comment fixes

---------

Co-authored-by: Frojdi Dymylja <33157909+fdymylja@users.noreply.github.com>

* chore: apply v46 changes (#455)

* adding posthandler and reflection services

* updating `sdkerrors.Wrap` with `errorsmod`

* every cosmos message protobuf definition must be extended with a cosmos.msg.v1.signer option to signal the signer fields

* better error for when post handler fails

* running go mod tidy

* registering query server after module init

---------

Signed-off-by: Spoorthi <9302666+spoo-bar@users.noreply.github.com>

* chore: apply v47 changes (#458)

* adding crisis module store key

* adding crisis module

* adding consensus module

* adding group module

* adding nft module

* fixing the merge

* fixing gentx decoding

* adding consensusparamtypes.StoreKey storekey && fixing staking keeper

---------

Signed-off-by: Spoorthi <9302666+spoo-bar@users.noreply.github.com>

* feat: bumping archway wasmd to v0.42.0 (#460)

* updating to wasmd fork 0.42.0

* adding capability cosmwasm_1_4

* updating v1.4 libwasmvm checksums

* chore: fixing linter errors and tests (#463)

* fixing lint issues

* fixing x/tracking tests

* fixing app_test.go

* replacing gov v1beta1 stuff with gov v1

* fixig app-upgrade-test

txRes.Data is deprecated in favour of txRes.MsgResponses

* removing depracated field usage

* fixing ibctm not setup

* making the flat fees e2e test simpler

* fixing interchaintest chain upgrade

* fixing TestTxFees thanks @fdymylja 🎉

* fixing more tests with initgenesis failure

* fixing testcase to be compatible with this change cosmos/cosmos-sdk#9651

* removing the ante.DeductFeeDecorator as we alreayd have a custom one

* fixing TestRewardsFlatFees

* undo the mintkeeper fix

* feat: adding upgrade handlers for sdk v47 (#464)

* adding missing consensus keeper

* adding upgrade handler stuff

* oops messed the merge. fixing that

---------

Signed-off-by: Spoorthi <9302666+spoo-bar@users.noreply.github.com>

* feat: enabling rosetta  (#466)

adding rosetta

* fix: updating logic for how we deal with infiniteGasMeter (#467)

* handling behavior change in infinitegasmeter.Limit()

* reflect no mo

* feat: x/rewards param store migration (#468)

* adding new msg proto

* implementing UpdateParams msg

* adding tests

* adding module migrations forx/rewards

* adding x/rewards to upgrade handlers

* adding migratestore test

* fixing lint

* addressing pr review comments

* Update CHANGELOG.md

* feat: bumping archway wasmd to v0.43.0 (#470)

* bumping wasmd fork to to v0.43.0

* updating wasmvm checksum

* removing wasmd deprecated proposals as they arent in the codebase anymore

* Updating changelog

* test: fixing ibc conformance test (#469)

fixing ibc conformance test. the custom genesis values are needed just for upgrade test

---------

Signed-off-by: Spoorthi <9302666+spoo-bar@users.noreply.github.com>
Co-authored-by: Frojdi Dymylja <33157909+fdymylja@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants