fix(ParasService): adjust endpoint to use historicApi, fix endingOffset bug #735
Conversation
src/services/paras/ParasService.ts
Outdated
| historicApi: ApiDecoration<'promise'>, | ||
| now: BN | ||
| ): BN { | ||
| const leasePeriod = historicApi.consts.slots.leasePeriod as BlockNumber; | ||
| return now.div(leasePeriod); |
There was a problem hiding this comment.
We actually need to adjust this due to this PR: paritytech/polkadot#3980
So I think it needs to be (now - lease_offset) / lease_period
There was a problem hiding this comment.
Ahhh okay sweet, thanks for the link. If I am seeing this correctly it is coming from this https://github.com/paritytech/polkadot/blob/master/runtime/common/src/crowdloan.rs#L1015
There was a problem hiding this comment.
@emostov updated if. Could you take a look when you get a chance
Co-authored-by: Zeke Mostov <[email protected]>
| const offset = | ||
| (historicApi.consts.slots.leaseOffset as BlockNumber) || BN_ZERO; | ||
|
|
||
| return now.sub(offset).div(leasePeriod); |
There was a problem hiding this comment.
This needs to be updated slightly. There is an edge case where offset > now and we decided to consider this has have no lease period index. In the substrate code this is represented by None. Here is the line paritytech/polkadot@3668966#diff-3e19d8a242311f202d80bcf9998ffdc22ddd933ed340f308276f353f137e2304R444
So basically it says "if now - offset is less than 0, return None". So we need a update for the logic that uses this function to basically say there is no lease period at now and we could add the first lease period start at block offset.
(This logic which got added to the substrate PR a bit late).
There was a problem hiding this comment.
(You can see the changes for this in substrate commit paritytech/polkadot@3668966)
src/services/paras/ParasService.ts
Outdated
| const leasePeriodIndex = currentLeasePeriodIndex + idx; | ||
| const leasePeriodIndex = currentLeasePeriodIndex | ||
| ? currentLeasePeriodIndex.toNumber() + idx | ||
| : 0; |
There was a problem hiding this comment.
Not sure what this should be returning.
rel: #698
This PR refactors the the ParasService to use a historicApi for querying the chain.
Bug Fix
There was a bug for
/experimental/paras/auctions/current?at=8400000where the incorrectendingOffsetwas being calculated and passed intoauctions.winningwhich would return the following:The endingOffset for block 8400000 should be 1478008, but we were receiving 412.
I used the implementation inside of
@polkadot/appsin order to fix the current bug.It also takes into account an edgecase where if the current now block substracted by the offset is less than zero than the leasePeriodIndex will be
null.