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

Ported mortal era encoding from its Rust implementation #172

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lyk0rian
Copy link

@lyk0rian lyk0rian commented Jun 23, 2021

I ported the mortal era encoding from its Rust implementation, which can be found there
https://github.com/paritytech/substrate/blob/c2ec5bc8f12bb5a084b976f2dc1280796e9c1b23/primitives/runtime/src/generic/era.rs#L66

Mortal era encoding can be tricky. What a good thing that we now have a GOlang implementation.

@lyk0rian lyk0rian changed the title ported mortal era encoding from its rust implementation Ported mortal era encoding from its rust implementation Jun 23, 2021
@lyk0rian lyk0rian changed the title Ported mortal era encoding from its rust implementation Ported mortal era encoding from its Rust implementation Jun 23, 2021
@codecov
Copy link

codecov bot commented Jun 23, 2021

Codecov Report

Merging #172 (9dfe2c1) into master (4fec3b3) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #172      +/-   ##
==========================================
+ Coverage   58.49%   58.61%   +0.11%     
==========================================
  Files          98       98              
  Lines        3602     3612      +10     
==========================================
+ Hits         2107     2117      +10     
  Misses       1081     1081              
  Partials      414      414              
Impacted Files Coverage Δ
types/extrinsic_era.go 74.19% <100.00%> (+12.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fec3b3...9dfe2c1. Read the comment docs.

@vedhavyas
Copy link
Contributor

@lyk0rian this is great! Could you also add an example on how Mortal era could be added to extrinsic instead of immortal and verify that works and also add a failing case ?

The changes looks good on first view. Will take another look again soon. Thanks!

@vedhavyas vedhavyas self-requested a review June 23, 2021 17:34
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