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.

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

Jump to bottom

feat: add era to blocks/{blockId} response for extrinsics #685

Merged
merged 15 commits into from
Oct 2, 2021
Merged

feat: add era to blocks/{blockId} response for extrinsics #685

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8b9cba6
fix: add eraInfo as a field in extrinsics response
TarikGul Sep 29, 2021
122f376
docs: update docs with GenericExtrinsicEra
TarikGul Sep 29, 2021
90c0903
fix: switch eraInfo to era
TarikGul Sep 29, 2021
ac049f1
docs: update docs
TarikGul Sep 29, 2021
294b250
fix: refactor
TarikGul Sep 29, 2021
c01bb8f
fix: lint
TarikGul Sep 29, 2021
602852b
fix: update mock data, and adjust unit tests
TarikGul Sep 29, 2021
91400cc
docs: update GenericExtrinsicEra description
TarikGul Sep 29, 2021
dd32594
Update docs/src/openapi-v1.yaml
TarikGul Sep 30, 2021
1cbba06
docs: adjust docs
TarikGul Sep 30, 2021
2bab240
docs: fix merge conflict
TarikGul Sep 30, 2021
39f08f3
tests(e2e): update kusama e2e test responses for /blocks
TarikGul Sep 30, 2021
9f85f45
tests(e2e): update the polkadot e2e tests for /blocks
TarikGul Sep 30, 2021
29425d8
tests(e2e): update westend e2e responses for /blocks
TarikGul Sep 30, 2021
c335892
fix: space at end of file
TarikGul Oct 1, 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
2 changes: 1 addition & 1 deletion docs/dist/app.bundle.js
View file
Open in desktop

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions docs/src/openapi-v1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1736,6 +1736,8 @@ components:
format: hex
info:
$ref: '#/components/schemas/RuntimeDispatchInfo'
era:
$ref: '#/components/schemas/GenericExtrinsicEra'
events:
type: array
description: An array of `SanitizedEvent`s that occurred during extrinsic
Expand Down Expand Up @@ -1793,6 +1795,23 @@ components:
trieIndex:
type: string
format: unsignedInteger
GenericExtrinsicEra:
type: object
description: |
The return value for eraInfo can either be `mortalEra`, or `immortalEra` and is represented as an enum in substrate. `immortalEra` meaning
the transaction is valid forever. `mortalEra` consists of a tuple containing a period and phase.
ex: `"{"mortalEra": ["64", "11"]}"`. The Period is the period of validity from the block hash found in the signing material.
The Phase is the period that this transaction's lifetime begins (and, importantly,
implies which block hash is included in the signature material).
Comment on lines +1803 to +1805
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this is a bit dense to parse. Is this correct? The first parameter, 64 in the example, is the Period and indicates the number of blocks for which the extrinsic is valid, starting from the block the extrinsic was signed? But what is the Phase? The wording "The Phase is the period" is confusing to me!

Copy link
Collaborator

@jsdw jsdw Sep 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This always confuses me in the documentation too. I assumed a mortal era was just a block number, and a number of blocks it'd be valid from after that (and the signature material contains the block hash so we can verify that the block numbered is what you'd expect), but I don't think I'm quite right (I'm not sure what a "phase" is)?!

Copy link
Collaborator

Choose a reason for hiding this comment

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

(to build on this; iirc we don't pass a block hash at all in the transaction; we just sign it against a block hash, so you need to know the block number you'll be getting the hash of to verify the signature I think?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the runtime keeps a mapping of BlockNumber => BlockHash for the last BlockHashCount blocks: https://github.com/paritytech/polkadot/blob/v0.9.10/runtime/common/src/lib.rs#L81

The mortality is a number of blocks, but it's encoded in one byte, so it's 2 ** mortality. Front ends abstract this by letting you "choose" a validity period, but if you choose e.g. 50 blocks it would actually be mortal for 64.

properties:
mortalEra:
type: string
nullable: true
immortalEra:
type: string
nullable: true
default: ['0x00']
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand this correctly, this is the only possible value for the immortal case? So rather than a default, it's just the hard coded value that must be there for Reasons™? If that is correct, can we get rid of it somehow, and have just immortalEra?

Copy link
Member Author

@TarikGul TarikGul Sep 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes totally, correct. I agree it should just be immortalEra in this case. A description explaining that the value is hardcoded or will always be '0x00' would possibly suffice?

Copy link
Collaborator

@jsdw jsdw Oct 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

One way to represent the return types I guess could be:

era: {
   type: "immortal"
}

or

era: {
   type: "mortal",
   period: 64,
   phase: 12
}

(with a typescript type like { type: "immortal" } | { type: "mortal", period: number, phase: number }. I'm pretty sure that can be represented in openAPI as well but I'd have to remind myself how!)

Copy link
Contributor

Choose a reason for hiding this comment

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

A description explaining that the value is hardcoded or will always be '0x00' would possibly suffice?

That's ok for this PR. Perhaps add an issue to investigate James' suggestion?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sweet, so I added a concise description for immortal -> Hardcoded constant '0x00'. I do like the idea of the return type being as @jsdw put it. The one reason I like it is the natural response from substrate has mortal being iterable, and immortal just being a string. Having different data structures in the same response field is a little odd for our circumstances.

I do think for now it might be wise to keep the response as close to substrate as possible. The idea being if anything were to change in substrate (low likely hood), the little logic there is for getting era wont change.

example: "{\"immmortalEra\": ['0x00']}"
TarikGul marked this conversation as resolved.
Show resolved Hide resolved
NodeNetwork:
type: object
properties:
Expand Down
4 changes: 2 additions & 2 deletions src/services/blocks/BlocksService.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,13 @@ describe('BlocksService', () => {
logs: [
{
preRuntime: [
'0x45424142',
'0x42414245',
'0x036d000000f4f4d80f00000000ec9bd8e2d0368c97f3d888837f7283bbe08266869eb613159db547905026c2502a70f168b9ffcc233344005d11ebecd166769200d270a2eaa642118a00acb708a0487a440b0caf3dd5c91ab173e80ddfe5735ef8b938ea87a6105a1161612707',
],
},
{
seal: [
'0x45424142',
'0x42414245',
'0xae78514e1de84a7d32e55b9b652f9d408ab1f7b4bfdbf6b2fad9cad94a91b86b0161cabf08f5ae1d3a1aa4993e2d96d56c94b03cee0898ccb8385a546084f88b',
],
},
Expand Down
4 changes: 3 additions & 1 deletion src/services/blocks/BlocksService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ export class BlocksService extends AbstractService {
typeof events === 'string' ? block.registry : events.registry;

return block.extrinsics.map((extrinsic) => {
const { method, nonce, signature, signer, isSigned, tip } = extrinsic;
const { method, nonce, signature, signer, isSigned, tip, era } =
extrinsic;
const hash = u8aToHex(blake2AsU8a(extrinsic.toU8a(), 256));
const call = registry.createType('Call', method);

Expand All @@ -375,6 +376,7 @@ export class BlocksService extends AbstractService {
tip: isSigned ? tip : null,
hash,
info: {},
era,
events: [] as ISanitizedEvent[],
success: defaultSuccess,
// paysFee overrides to bool if `system.ExtrinsicSuccess|ExtrinsicFailed` event is present
Expand Down
4 changes: 2 additions & 2 deletions src/services/test-helpers/mock/data/block789629.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
"logs": [
{
"PreRuntime": [
1161969986,
1111573061,
"0x036d000000f4f4d80f00000000ec9bd8e2d0368c97f3d888837f7283bbe08266869eb613159db547905026c2502a70f168b9ffcc233344005d11ebecd166769200d270a2eaa642118a00acb708a0487a440b0caf3dd5c91ab173e80ddfe5735ef8b938ea87a6105a1161612707"
]
},
{
"Seal": [
1161969986,
1111573061,
"0xae78514e1de84a7d32e55b9b652f9d408ab1f7b4bfdbf6b2fad9cad94a91b86b0161cabf08f5ae1d3a1aa4993e2d96d56c94b03cee0898ccb8385a546084f88b"
]
}
Expand Down
4 changes: 2 additions & 2 deletions src/services/test-helpers/mock/data/blocks789629Fork.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
"logs": [
{
"PreRuntime": [
1161969986,
1111573061,
"0x036d000000f4f4d80f00000000ec9bd8e2d0368c97f3d888837f7283bbe08266869eb613159db547905026c2502a70f168b9ffcc233344005d11ebecd166769200d270a2eaa642118a00acb708a0487a440b0caf3dd5c91ab173e80ddfe5735ef8b938ea87a6105a1161612707"
]
},
{
"Seal": [
1161969986,
1111573061,
"0xae78514e1de84a7d32e55b9b652f9d408ab1f7b4bfdbf6b2fad9cad94a91b86b0161cabf08f5ae1d3a1aa4993e2d96d56c94b03cee0898ccb8385a546084f88b"
]
}
Expand Down
3 changes: 3 additions & 0 deletions src/services/test-helpers/responses/blocks/block789629Extrinsic.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
"tip": null,
"hash": "0xcf52705d1ade64fc0b05859ac28358c0770a217dd76b75e586ae848c56ae810d",
"info": {},
"era": {
"immortalEra": "0x00"
},
"events": [
{
"method": {
Expand Down
Loading