search

polkadot-js / api

Promise and RxJS APIs around Polkadot and Substrate based chains via RPC calls. It is dynamically generated based on what the Substrate runtime provides in terms of metadata.
Apache License 2.0
1.07k stars 354 forks source link

fix: check if minimumPeriod is zero #5827

Closed pepoviola closed 8 months ago

pepoviola commented 8 months ago

Fix #5826

Add check to ensure we don't use 0.

TarikGul commented 8 months ago

Since the augmented types dont have the aura consts yet, you will have to type case a custom type on api.consts.aura that sets slotDuration as a key value to AugmentedConsts<"rxjs">.aura. This can be all done locally inside of the api-derive.

TarikGul commented 8 months ago

An example of this could be:

interface Aura {
  slotDuration: u64 & AugmentedConsts<'rxjs'>;
}
...
const auraApi = api.consts['aura'] as unknown as Aura;
pepoviola commented 8 months ago

Since the augmented types dont have the aura consts yet, you will have to type case a custom type on api.consts.aura that sets slotDuration as a key value to AugmentedConsts<"rxjs">.aura. This can be all done locally inside of the api-derive.

Oks, does make sense to use the old version

(!api.consts.timestamp?.minimumPeriod.isZero() && api.consts.timestamp.minimumPeriod.muln(2) ) ||

until https://github.com/paritytech/polkadot-sdk/pull/3732 is merged so we can use the aura const?

TarikGul commented 8 months ago

Since the augmented types dont have the aura consts yet, you will have to type case a custom type on api.consts.aura that sets slotDuration as a key value to AugmentedConsts<"rxjs">.aura. This can be all done locally inside of the api-derive.

Oks, does make sense to use the old version

(!api.consts.timestamp?.minimumPeriod.isZero() && api.consts.timestamp.minimumPeriod.muln(2) ) ||

until paritytech/polkadot-sdk#3732 is merged so we can use the aura const?

If we can maintain the logic that fixes this and patch with this then sure that is totally acceptable!

pepoviola commented 8 months ago

Few nits, but looks amazing. Once the suggestions are fixed this should be good to go. Also needs a yarn lint once the following is fixed.

Thanks for review it, I made those changes 👍

polkadot-js-bot commented 8 months ago

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.