Skip to content

fix(ParasService): adjust endpoint to use historicApi, fix endingOffset bug #735

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.

Sign up for GitHub

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

Merged
TarikGul merged 17 commits into from
Nov 10, 2021
Merged

fix(ParasService): adjust endpoint to use historicApi, fix endingOffset bug #735

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0592f95
add historicApi to crowdloans-info
TarikGul Oct 26, 2021
85377b6
add assertQueryModule check
TarikGul Oct 26, 2021
6302683
add historicApi to crowdloans
TarikGul Oct 26, 2021
47d48f6
add historicApi to leases-info and leases/current
TarikGul Oct 26, 2021
dca40f8
add historicApi to auctions-current
TarikGul Oct 26, 2021
c408b4c
add historicApi to paras
TarikGul Oct 27, 2021
202036b
fix endingOffset bug
TarikGul Oct 27, 2021
bb569dc
adjust tests to reflect historicApi
TarikGul Oct 27, 2021
0022526
cleanup
TarikGul Oct 27, 2021
24148e0
inline docs
TarikGul Oct 27, 2021
18ae042
lint
TarikGul Oct 27, 2021
5f1109c
Update src/services/paras/ParasService.ts
TarikGul Oct 27, 2021
7415f9f
adjust lease_period_index
TarikGul Nov 2, 2021
af7e704
merge master
TarikGul Nov 10, 2021
dfa5f0f
lease period index edgecase
TarikGul Nov 10, 2021
f632e6a
adjust leasePeriodIndex to be null not BN_ZERO
TarikGul Nov 10, 2021
c51ea90
update docs
TarikGul Nov 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/services/paras/ParasService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,13 @@ export class ParasService extends AbstractService {
const currentLeasePeriodIndex = this.leasePeriodIndexAt(
historicApi,
blockNumber
).toNumber();
);

leasesFormatted = leases.reduce((acc, curLeaseOpt, idx) => {
if (curLeaseOpt.isSome) {
const leasePeriodIndex = currentLeasePeriodIndex + idx;
const leasePeriodIndex = currentLeasePeriodIndex
? currentLeasePeriodIndex.toNumber() + idx
: 0;
Copy link
Copy Markdown
Member Author

@TarikGul TarikGul Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this should be returning.

Copy link
Copy Markdown
Contributor

@emostov emostov Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be null

const lease = curLeaseOpt.unwrap();
acc.push({
leasePeriodIndex,
Expand Down Expand Up @@ -330,14 +332,16 @@ export class ParasService extends AbstractService {

const leasePeriod = historicApi.consts.slots.leasePeriod as BlockNumber;
const leasePeriodIndex = this.leasePeriodIndexAt(historicApi, blockNumber);
const endOfLeasePeriod = leasePeriodIndex.mul(leasePeriod).add(leasePeriod);
const endOfLeasePeriod = leasePeriodIndex
? leasePeriodIndex.mul(leasePeriod).add(leasePeriod)
: BN_ZERO;

return {
at: {
hash,
height: blockNumber.toString(10),
},
leasePeriodIndex,
leasePeriodIndex: leasePeriodIndex ? leasePeriodIndex : BN_ZERO,
endOfLeasePeriod,
currentLeaseHolders,
};
Expand Down Expand Up @@ -399,11 +403,16 @@ export class ParasService extends AbstractService {
private leasePeriodIndexAt(
historicApi: ApiDecoration<'promise'>,
now: BN
): BN {
): IOption<BN> {
const leasePeriod = historicApi.consts.slots.leasePeriod as BlockNumber;
const offset =
(historicApi.consts.slots.leaseOffset as BlockNumber) || BN_ZERO;

// Edge case, see https://github.com/paritytech/polkadot/commit/3668966dc02ac793c799d8c8667e8c396d891734
if (now.toNumber() - offset.toNumber() < 0) {
return null;
}

return now.sub(offset).div(leasePeriod);
Copy link
Copy Markdown
Contributor

@emostov emostov Nov 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Contributor

@emostov emostov Nov 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(You can see the changes for this in substrate commit paritytech/polkadot@3668966)

}

Expand Down