-
Notifications
You must be signed in to change notification settings - Fork 0
Passing the era #2
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
base: master
Are you sure you want to change the base?
Conversation
packages/types/src/types.ts
Outdated
| } | ||
|
|
||
| // export interface IExtrinsicEra extends U8a { | ||
| export interface IExtrinsicEra extends EnumType<ImmortalEra | MortalEra> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IExtrinsicEra should not be correlated with a particular implementation.
i think you need to define IExtrinsicEra first, then let your ExtrinsicEra implement it.
| // of the Apache-2.0 license. See the LICENSE file for details. | ||
|
|
||
| import ExtrinsicEra from './ExtrinsicEra'; | ||
| import U64 from "@polkadot/types/primitive/U64"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use relative path
|
|
||
| it('decodes a Extrinsic Era from hexstring as mortal', () => { | ||
| const mortalIndex = 1; | ||
| const extrinsicEra = new ExtrinsicEra('0x0401000001010b00000000',mortalIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when decode from ua8, no extra information is needed. same for the rest.
|
|
||
| it('decodes a Extrinsic Era from Object with phase & period as mortal instance', () => { | ||
| const mortalIndex = 1; | ||
| const extrinsicEra = new ExtrinsicEra({enabled: true, period: new U64(32), phase: new U64(16)}, mortalIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we don't need enabled as part of parameter. if user don't want the extrinsic expire, they can pass new ImmortalEra().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if user want to create a mortal era, they should use new MortalEra();
| try { | ||
| const nonce = await api.query.system.accountNonce(keyring.dave.address()); | ||
| const signedBlock = await api.rpc.chain.getBlock(); | ||
| const exERA = new ExtrinsicEra(new Uint8Array([248, 13]), 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this test is happy path, you need to make sure the era you construct is always valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for example you should try to create a era which is valid from current to next 10 blocks. (make 10 a varible)
| }); | ||
|
|
||
|
|
||
| it.only('encode an Extrinsic Era from Object with blocknumber & period as mortal instance', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove only
| if (period.toNumber() >= 4 && phase < period.toNumber()) { | ||
| return [new U8(period), new U8(phase)]; | ||
| } | ||
| return [new U8(), new U8()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a default return, so this line can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in which case the code will reach this line. maybe we should throw an error?
| const current = value.startBlockNumber; | ||
| const period = value.endBlockNumber.toNumber() - value.startBlockNumber.toNumber(); | ||
| let calPeriod = Math.pow(2, Math.ceil(Math.log2(period))); | ||
| calPeriod = Math.max(calPeriod, 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can put max and min in one row.
| const period = new U64(this.period); | ||
| const phase = new U64(this.phase); | ||
| const quantize_factor = Math.max(period.toNumber() >> 12, 1); | ||
| const trailingZeros: any = this.getTrailingZeros(period); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try remove any
| return new Uint8Array([second, first]); | ||
| } | ||
|
|
||
| getTrailingZeros(period: U64): U32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why return as U32? don't see the point
|
|
||
|
|
||
| return new Uint8Array([0]); | ||
| birth(current: U64) : U64{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can just return number.
packages/types/src/types.ts
Outdated
| era: IExtrinsicEra; | ||
| } | ||
|
|
||
| export interface IExtrinsicEra { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a proper interface for Era
| if (value) { | ||
| const u8a = u8aToU8a(value); | ||
| getTrailingZeros(period: U64) { | ||
| let zeros = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const zero: number[] = [];
| return u8a.subarray(0, (u8a[0] === 0) ? 1 : 2); | ||
| while (periodN % 10 == 0) { | ||
| periodN = periodN /= 10; | ||
| zeros += 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zeros.push(0)
| const nonce = await api.query.system.accountNonce(keyring.dave.address()); | ||
| const signedBlock = await api.rpc.chain.getBlock(); | ||
| const currentHeight = signedBlock.block.header.number; | ||
| const exERA = new ExtrinsicEra({ current: new U64(currentHeight+10), period: new U64(10) }, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i thought there's no need for currentHeight + 10
| return zeros.length; | ||
| } | ||
|
|
||
| birth (current: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comments for birth and death
| ); | ||
| super(value); | ||
|
|
||
| assert(this.eq(VALID_IMMORTAL), `IMMORTAL: expected ${VALID_IMMORTAL.toHex()}, found ${this.toHex()}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you try this.eq([0]) if this works, then you can just let VALID_IMMORTAL = [0]
ERA changes