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

Modify extractAuthor to support Moonbeam #3082

Closed
wants to merge 2 commits into from
Closed

Modify extractAuthor to support Moonbeam #3082

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd443f8
modify extractAuthor to support Moonbeam
joelamouche Jan 20, 2021
256c60a
typo
joelamouche Jan 20, 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
28 changes: 17 additions & 11 deletions packages/api-derive/src/type/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,29 @@
import type { AccountId, Digest } from '@polkadot/types/interfaces';

export function extractAuthor (digest: Digest, sessionValidators: AccountId[] = []): AccountId | undefined {
const [pitem] = digest.logs.filter(({ type }) => type === 'PreRuntime');
let author;
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I would swap the order around - the most-used is PreRuntime (and most recent), so it should go first.

Additionally we have lost the return engine.extractAuthor(data, sessionValidators); from the citems now. In reality the specific log for Moonbeam should probably be in there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I would swap the order around - the most-used is PreRuntime (and most recent), so it should go first

If you look at the logs in moonbeam-foundation/moonbeam#159 (comment) you'll see that Moonbeam also has a PreRuntime log but it doesn't work. So we should first check for Consensus before we look at PreRuntime.

Additionally we have lost the return engine.extractAuthor(data, sessionValidators); from the citems now. In reality the specific log for Moonbeam should probably be in there?

return engine.extractAuthor(data, sessionValidators) doesn't work for Moonbeam. Since all other uses of Consensus have been removed, I assumed it would be simple to just drop it. I can reestablish it for non-h160 addresses if you think it's useful

Copy link
Contributor

Choose a reason for hiding this comment

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

Moonbeam also has a PreRuntime log

That's only true when you're talking about the standalone node with Aura. The parachain node (the primary moonbeam product) does not have a PreRuntime digest, although it may in the future). For example, look at the live testnet.

image

Copy link
Member

@jacogr jacogr Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For instance, by just returning here we have actually broken Kulupu which also uses the Consensus digest and also has no validators, it is POW.

The actual extraction needs to happen here - https://github.com/polkadot-js/api/blob/master/packages/types/src/generic/ConsensusEngineId.ts

(Aura, Babe & Kulupu POW is done there)

Copy link
Member

Choose a reason for hiding this comment

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

TL;DR this logic needs to be kept in-tact, it will determine the type of digest and defer to the ConsensusEngineId for extracting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But if we keep this logic, this won't work for the standalone, which would have a PreRuntime log, but where the relevant information is in the Consensus log

Copy link
Member

@jacogr jacogr Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The logic is as follow, directl from the source, commented here ...

  // try to find any PreRuntime digests here  
  const [pitem] = digest.logs.filter(({ type }) => type === 'PreRuntime');

  if (pitem) {
   // if we have some PreRuntime logs, follow this path
    ...
  } else {
    // we didn't have any of the previous type, now try to find Consensus logs
    const [citem] = digest.logs.filter(({ type }) => type === 'Consensus');
    ...
  }

So if doesn't require PreRuntime, it is one of the two paths.

Copy link
Member

@jacogr jacogr Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm happy to swap the oder above for the case where there are both, as long as we don't try and make the extraction here as well.

const citems = digest.logs.filter(({ type }) => type === 'Consensus');

// extract from the substrate 2.0 PreRuntime digest
if (pitem) {
const [engine, data] = pitem.asPreRuntime;
if (citems.length > 0) {
citems.forEach((citem) => {
// extract author from the consensus (substrate 1.0, digest)
const [, data] = citem.asConsensus;

return engine.extractAuthor(data, sessionValidators);
if (data.length === 20) {
// This is used by Moonbeam with an h160 address for the author
author = data.toString();
}
});
} else {
const [citem] = digest.logs.filter(({ type }) => type === 'Consensus');
// Aura and Babe use PreRuntime digest now
const [pitem] = digest.logs.filter(({ type }) => type === 'PreRuntime');

// extract author from the consensus (substrate 1.0, digest)
if (citem) {
const [engine, data] = citem.asConsensus;
if (pitem) {
const [engine, data] = pitem.asPreRuntime;

return engine.extractAuthor(data, sessionValidators);
author = engine.extractAuthor(data, sessionValidators);
}
}

return undefined;
return author;
}