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

Fixes for compatibility with latest Kusama runtime #12

Merged

Conversation

mario-sangar
Copy link
Contributor

@mario-sangar mario-sangar commented May 24, 2021

Fixes for compatibility with latest Kusama runtime

Description

I discovered that the list command was returning incorrect values for rewards on Kusama and Westend, but not on Polkadot. Going down the rabbit hole investigating that, I made "some" changes on the code and I include them all in here.

Changes

  • Added a standard .gitignore for Python projects.
  • Fixed some commands on the README and made the results be with exact decimals (spoiler: as the tool returns them now).
  • Bumped the substrate-interface dependency to 13.3
  • Command list
    • Accounts don't need decoding now, so we use SS58 formatted addresses for everything
    • Removed HistoryDepth call, as it's not used (the depth is obtained from the config)
    • Using ActiveEra instead of CurrentEra to avoid claiming rewards for an unfinished era.
    • Renamed (only internally) the --unclaimedflag as args.only_unclaimed for readability.
    • Using f-strings for simpler formatting of strings.
    • Order results by era in DESC order.
    • Fixed length of substrate.token_decimals for balances
    • Fixed typos on paymemt -> payment.
  • Command pay
    • Removed HistoryDepth call, as it's not used (the depth is obtained from the config)
    • Max depth default value is now 84 (previously 82)
    • Using ActiveEra instead of CurrentEra to avoid claiming rewards for an unfinished era
    • Step-by-step logging
    • Checking current account balance of the issuer before submitting a extrinsic, exiting if it's not enough
    • Keypair generation now also reads from CLI arguments (previously was only from config). Even with this feature now enabled, I think it should be only used for development purposes. Secrets on the CLI may become visible for third parties, better use config files.
    • Waiting for the batch extrinsic to be in a block before exiting.
    • Logging the result of the extrinsic, with data such as its hash, the block hash, the fee or whether it was correct.
    • Mapping from network to ss58_format for correct keypair generation.

Suggestion to bump version

I think these changes don't affect the usage of the tool from the user's perspective, but I would suggest to bump the version of the tool to 1.1.0.

Let me know what you think about this and the changes above, I hope you find them useful 😁

I just saw that @arjanz has made a PR. I think his changes are also included in this PR so far, but please, take a look at both :)

- Account decoding not needed, now using ss58 enconding all the way
- Removing history_depth call (depth from configuration)
- Using ActiveEra instead of CurrentEra (prevents including an
unfinished era)
- Rename arg internally as "only_unclaimed"
- Using f-string's for logging results
- Order results per era in DESC order
- Length of result fixed at token_decimals
- Fix typo on "eras_paymemt_info"
- Removing history_depth call (depth from configuration)
- Using 84 as default depth value (previously 82)
- Using ActiveEra instead of CurrentEra (prevents including an
unfinished era)
- Included step by step logging (using f-string's)
- Checking current balance before submitting extrinsic
- Getting keypair config from args (previously only from config)
- Waiting for extrinsic to be in block
- Logging result extrinsic receipt info
- Add mapping of ss58 address format by network
@mario-sangar
Copy link
Contributor Author

mario-sangar commented May 25, 2021

The mapping of network to SS58 address format was extracted from the Substrate repo:

https://github.com/paritytech/substrate/blob/3c2bd9a6ba1052daa8711dfb22b902e7591ab690/primitives/core/src/crypto.rs#L479

@mario-sangar mario-sangar force-pushed the mariops-fixes-for-kusama-runtime-9010 branch from 041121b to 0504d6c Compare May 25, 2021 12:18
@apuigsech apuigsech merged commit 57b6405 into stakelink:master May 26, 2021
@mario-sangar mario-sangar deleted the mariops-fixes-for-kusama-runtime-9010 branch May 28, 2021 13:47
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.

2 participants