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

v44 SDK Upgrade Overview (WIP) #154

Open
blewater opened this issue Sep 28, 2021 · 12 comments
Open

v44 SDK Upgrade Overview (WIP) #154

blewater opened this issue Sep 28, 2021 · 12 comments
Milestone

Comments

@blewater
Copy link
Contributor

  1. SDK API breaking changes
    Impact
    Build breaks and business impact needs to be assessed i.e. whether updated API is a 1-1 replacement.
    Action Item
    These API items need to be consulted as we migrate the codebase. v44 Api Changes

  2. State Upgrade Details
    Impact
    Banking and evidence migrate to v44 state format.
    Action Item
    . Write the in-store migration plan and plan the chain upgrade along.
    . Test a smooth IBC upgrade.

  3. e-Money v42 multisig fix.
    Impact
    Needs to be migrated to v44.
    Action Item
    Check whether Henrik's fix has been backported to v44 otherwise, fork v44 and migrate fix and check against new ADR-028 addresses. See below.

  4. ADR-028 Addresses i.e., secp256r1 or 32 byte addresses. doc updates
    Impact
    This is extending the previous Address scheme with new longer address types that would need to be integrated/tested with emd, wallet, test, and live keystores. Not clear on the status of hardware devices support.
    Action
    Adding a comprehensive testing suite and testing support across the board.

  5. Upgrade modules to support in-place store migrations In store migration
    Impact
    Module code changes.
    Action Item
    None immediately unless the new functionality is desired

  6. IBC has moved to a new repo.
    Impact
    The build breaks.
    Action Items
    Migrate existing IBC imports to the new IBC repo dependency. Check against other issues.

  7. New proto / module folder layout changes
    Impact
    New streamlined module folder structure.
    Action Items
    The existing folder layout appears upward compatible, but we may have to compile against a revamped third_party proto folder.

  8. Removal of legacy Rest endpoints.
    Impact:
    Likely none because we are using the gRPC gateway Rest Api already and had removed the legacy endpoints.
    Action Item:
    Check the wallet endpoints against this table.
    New rest endpoints

  9. Cli Changes https://github.com/cosmos/cosmos-sdk/blob/release/v0.44.x/CHANGELOG.md#cli-breaking-changes
    Impact
    Minimal or none

Suggested em-ledger migration plan

  • Initially, one person defines a minimum functional build along with dependencies.
  • More than one of us can tackle any of the affected areas.
@blewater blewater added this to the SDK v44 milestone Sep 28, 2021
@blewater
Copy link
Contributor Author

@haasted
Copy link
Collaborator

haasted commented Sep 29, 2021

Excellent list!

Regarding point 8, we'll need to map out in what detail the REST endpoints are being used by the wallet and explorers.

@haasted
Copy link
Collaborator

haasted commented Sep 29, 2021

  1. Verify Ledger support
    Impact:
    Likely works, but the removal of all Amino structs is likely to cause changes.
    Action Item:
    Thorough testing of Ledger support must be done. Especially messages that are specific to our chain, eg. the market functionality.

@dualsystems
Copy link

  1. gRPC bug (/cosmos.tx.v1beta1.Service/Simulate gas auto) - Invalid simulation query on v0.40.1 cosmos/cosmos-sdk#8425

@blewater
Copy link
Contributor Author

blewater commented Oct 4, 2021

  1. the IBC module in v44+ adds the upgrade keeper as a dependency: IBC Core NewKeeper()

Sides notes: the integration documentation has not caught up with it yet. Configuring the keeper
Motivation: likely improved support for the Chain upgrade scenarios

Action Item: Investigate the IBC query requirements for the Upgrade state and whether to undertake similar state work as with the staking state history availability.

@blewater
Copy link
Contributor Author

blewater commented Oct 4, 2021

@faddat
Copy link
Contributor

faddat commented Oct 10, 2021

I can help with this.

In general I don't do fresh code on chains, but i have gotten really good at upgrades and the like.

one thing

stay away from 0.44.1

@blewater
Copy link
Contributor Author

stay away from 0.44.1

@faddat thanks for the heads up. Specific issue? wait for v0.44.2?

@haasted
Copy link
Collaborator

haasted commented Oct 11, 2021

@faddat : That sounds good! You're very welcome to take a look at the on-going upgrade work on https://github.com/e-money/em-ledger/tree/v44. Please let us know if you have any suggestions or things that you want to contribute. PRs are very welcome.

@haasted
Copy link
Collaborator

haasted commented Oct 12, 2021

Tested the IBC test setup located in networks/ibc. Seems to run without too many issues, although there is a warning about being unable to estimate gas. Increasing Hermes' clock_drift setting seemed to take care of that issue.

@blewater
Copy link
Contributor Author

although there is a warning about being unable to estimate gas.

Seems there are a couple of gas estimation related issues in Hermes i.e., fixed in v0.7.3 informalsystems/hermes#1354 and it's unclear whether it requires configuration adjustment as discussed in the issue

@faddat
Copy link
Contributor

faddat commented Jan 22, 2022

Hi, another thing -- probably best now to jump straight to v0.45.0 because it's got some performance improvements, and is state-breaking anyhow :).

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

No branches or pull requests

3 participants