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.

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

Client update based on Beefy protocol #56

Closed
livelybug opened this issue Jan 17, 2022 · 9 comments
Closed

Client update based on Beefy protocol #56

livelybug opened this issue Jan 17, 2022 · 9 comments

Comments

@livelybug
Copy link

livelybug commented Jan 17, 2022

In Beefy protocol, there's one data structure, MMR root, "one level higher" than block header. The MMR root of chainA is generated and broadcasted once every N new blocks are generated. After ChainA's light client on ChainB receives a tx containing a new MMR root, it will verify the MMR root, and then verify any of the N headers based on the verified MMR root asynchronously.

From the architectural perspective, is it a good idea to do the 2 asynchronous verifications in the existing function check_header_and_update_state?

Kindly reply if any comments. thank you.

@livelybug
Copy link
Author

In Beefy protocol, there's one data structure, MMR root, "one level higher" than block header. The MMR root of chainA is generated and broadcasted once every N new blocks are generated. After CahinA's light client on chainB receives a tx containing a new MMR root, it will verify the MMR root, and then verify any of the N headers based on the verified MMR root asynchronously.

From the architectural perspective, is it a good idea to do the 2 asynchronous verifications in the existing function check_header_and_update_state?

Kindly reply if any comments. thank you.

@andynog This is a possible architectural change integrating Beefy light client to substrate-ibc.

@adizere
Copy link
Contributor

adizere commented Jan 18, 2022

I'm not sure what you mean by "asynchronous". In general, all verification methods should be called synchronously, whenever the IBC module is processing a transaction containing a client update message.

Can you expand or detail on the sequence of actions you have in mind?

@livelybug
Copy link
Author

livelybug commented Jan 19, 2022

Let me try to describe more about the "asynchronous", which may not be accurate under this scenario:

  • Under Grandpa consensus, a Relayer submits an update client tx to ChainB after receiving an IbcEvent::NewBlock event from ChainA. ChainB will be able to verify the header in the update client tx immediately via function check_header_and_update_state. Then any tx belonging to the same header can also be verified by ChainB accordingly.

  • After applying Beefy protocol, ChainB cannot verify the header immediately after receiving, as ChainB may not have verified the MMR root the header belongs to.

    Which header belongs to which MMR root? The relayer receives MMR root via subscription subscribeJustifications. Let's say the message containing an MMR root is pushed to the relayer every 8 blocks, then headers of block height 1-8 can be verified via or "belong to" the MMR root of block number 8; headers of block height 1-16 can be verified via or "belong to" the MMR root of block number 16; headers of block height 1-24 can be verified via or "belong to" the MMR root of block number 24; ...

  • Therefore, under Beefy protocol, a header's verification may have to "wait" until the corresponding MMR root is verified. E.g., If ChainB receives ChainA's header of height 20, the header cannot be verified until ChainA's block height reaches 24 and the MMR root of height 24 is generated and relayed to ChainB. The "wait" makes "asynchronous".

@livelybug
Copy link
Author

A temporary solution for this issue:

@Wizdave97
Copy link

Wizdave97 commented May 5, 2022

@adizere the refactor of checkHeaderAndUpdateState being discussed here should be the same as the 02-client-refactor in ibc-go here https://github.com/cosmos/ibc-go/blob/02-client-refactor/modules/light-clients/07-tendermint/types/update.go.

The checkHeaderAndUpdateState needs to be refactored into component functions as done in ibc-go linked above, because of the reasons mentioned above.

@hu55a1n1
Copy link
Contributor

@livelybug can you confirm that the refactoring in PR informalsystems/hermes#2284 (specifically to the ClientDef::check_header_and_update_state() method here as mentioned by @Wizdave97 above) addresses this issue?

@DaviRain-Su
Copy link
Contributor

DaviRain-Su commented Aug 29, 2022

@adizere the refactor of checkHeaderAndUpdateState being discussed here should be the same as the 02-client-refactor in ibc-go here https://github.com/cosmos/ibc-go/blob/02-client-refactor/modules/light-clients/07-tendermint/types/update.go.

The checkHeaderAndUpdateState needs to be refactored into component functions as done in ibc-go linked above, because of the reasons mentioned above.

this link is break down.

@hu55a1n1
Copy link
Contributor

hu55a1n1 commented Sep 2, 2022

@DaviRain-Su, that branch has been deleted and superseded by the updated-02-client-refactor branch. I believe the refactoring being referred to is in this PR -> cosmos/ibc-go#1871, you will find the diff for modules/light-clients/07-tendermint/types/update.go there. The PR has since been merged and the branch deleted.

@livelybug
Copy link
Author

The MMR root update service is integrated into this branch

@hu55a1n1 hu55a1n1 transferred this issue from informalsystems/hermes Sep 29, 2022
shuoer86 pushed a commit to shuoer86/ibc-rs that referenced this issue Nov 4, 2023
Refactoring go test cases to be in json format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants