Modify extractAuthor to support Moonbeam #3082
Conversation
| @@ -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; | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
TL;DR this logic needs to be kept in-tact, it will determine the type of digest and defer to the ConsensusEngineId for extracting.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Replaced by #3108 |
|
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. |
This modification will allow displaying the author of a block for a parachain/blockchain that uses Consensus digest with an h160 address for the author.
@jacogr let me know if that covers all the other parachains