feat: add grandpa justifications mock rpc #695
Conversation
|
is this impl what we expect? @xlc |
|
weired, cannot pass test, the callback is only called once.. |
|
I'm afraid there's no use of fake data for these methods. These are required for beefy and needs to be real data |
|
It is not possible to have real data as we don't have the signing keys. So the user of those RPCs (i.e. the bridge relayer) needs to have a special mode to skip signature validation, which is the plan. |
| return { | ||
| at: hash, | ||
| proof: keys.map(() => ('0x' + '7'.repeat(64)) as HexString), | ||
| } |
There was a problem hiding this comment.
Is there any way we could get a real proof here ? As far as I know, for this we don't need signing keys.
There was a problem hiding this comment.
@serban300 I've implemented state_getReadProof to return proof of requested keys but the trie_state_root won't be the same as block trie_state_root. I don't know if correct hash is required and not sure if we can do that. Can you give this a try first before I look deeper into the implementation?
There was a problem hiding this comment.
@ermalkaleci thanks ! Yes, will try this. Just have to prioritize some reviews first, and then I will get to this. Hopefully this week.
There was a problem hiding this comment.
Checked. This doesn't work. I'm getting an error when validating the storage proof. We would need the trie_state_root to be the same as the block trie_state_root. Since I think that's how we're validating the proof. Would that be possible ?
|
|
||
| update = async (block: Block) => { | ||
| // Proof needs to be different for each block | ||
| callback(blake2AsHex(block.hash)) |
There was a problem hiding this comment.
I'm getting an error when receiving such a proof:
[Polkadot_to_BridgeHubKusama_Sync] 2024-03-21 20:22:21 +00 ERROR bridge Failed to read justification target from the Polkadot justifications stream: "decode failed with error Error { cause: Some(Error { cause: Some(Error { cause: None, desc: \"Not enough data to fill buffer\" }), desc: \"Could not decode `Commit::target_hash`\" }), desc: \"Could not decode `GrandpaJustification::commit`\" }"
The proof needs to be an encoded GrandpaJustification that contains at least the targetHash and targetNumber. Something like:
const justification: GrandpaJustification = {
round: 0,
commit: {
targetHash: block.hash,
targetNumber: block.number,
precommits: []
},
votesAncestries: []
};
And then it needs to be encoded.
Not sure how to instantiate such a type here. Can you take a look please ?
There was a problem hiding this comment.
@serban300 can you try latest commit?
There was a problem hiding this comment.
@serban300 do you have some instructions how I can test this?
There was a problem hiding this comment.
@ermalkaleci just some context first. I haven't tested it yet, but I don't think this will work. Looks like it returns a beefy justification from grandpa_subscribeJustifications if I understand correctly. These should be 2 different APIs: grandpa_subscribeJustifications and beefy_subscribeJustifications. Also I would define them in 2 separate files.
Another thing I should mention is that we don't use the beefy justifications yet for the Polkadot<>Kusama bridge. We have a beefy light client, but it's still a work in progress, we don't use it now and it's not a priority. We only use grandpa. So we don't need beefy_subscribeJustifications yet. I don't know about the team working at the Ethereum bridge. They use beefy, but I don't know if they plan to perform tests using chopsticks.
For grandpa_subscribeJustifications we need to return an encoded GrandpaJustification struct as mentioned in the previous comment. For the moment I think a good enough test would be just to decode it and see that the targetHash and targetNumber match the latest finalized block if that's possible.
There was a problem hiding this comment.
ok my bad. I was looking for justification on wested and I just found BEEFY. updating now
There was a problem hiding this comment.
@ermalkaleci Thanks ! Now I'm not getting any decoding error anymore. And if I comment the finality proof verification logic in the relayer and in the bridge hubs runtimes, updating the best header for the bridged relay chain works.
There was a problem hiding this comment.
Can you send me a link to the code you're disabling?
There was a problem hiding this comment.
Sure.
This is the disabled runtime code: serban300/polkadot-sdk@06dcf0a
And this is the disabled relayer code: serban300/parity-bridges-common@f334bdd
add