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

Make the domain client instantiation-aware #1716

Merged
merged 7 commits into from
Aug 1, 2023
Merged

Conversation

NingLin-P
Copy link
Member

@NingLin-P NingLin-P commented Jul 28, 2023

In the main branch, the domain client assumes that the domain chain will start at the genesis block of the consensus chain. This is not true in domain v2, where the domain chain is started after the domain is instantiated at the consensus chain, for the genesis domain it starts at consensus block #1 due to our workaround of the extension issue. This PR recorrects this assumption in the domain client and handles various edge cases properly.

The error we meet in the devnet:

2023-07-28 14:27:03 [Domain] Failed to process consensus block on startup error=Backend("Consensus hash for domain hash #0,0x8a3a…f13a not found")

is because the domain block processor handles the consensus block out of order (i.e. the first consensus block being processed is not block #1), this can happen if the consensus chain import block when the domain chain is still initializing:
https://github.com/subspace/subspace/blob/d2b394445acb5c8f1be3b62501f47e805a73ca9a/domains/client/domain-operator/src/domain_worker.rs#L97-L109

this PR's last commit (although very tiny) adds a test case to simulate this error (pick this commit to the main branch and run the test, which will result in the same error) and ensure it is fixed.

Code contributor checklist:

This value will be used in later commit

Signed-off-by: linning <[email protected]>
This value indicate the consensus block number that the domain instance first
instantiated

Signed-off-by: linning <[email protected]>
Before this commit, the domain client assume that the domain chain start
at the consensus chain's genesis block, but in domain v2, the domain chain
only start after it is instantiated at the consensus chain, namely started
at domain_created_at, this commit recorrect this assumption and handling
various edge cases properly

Signed-off-by: linning <[email protected]>
…le the consensus block even it is out of order

Signed-off-by: linning <[email protected]>
@NingLin-P NingLin-P force-pushed the domain-instance-fix branch from 6bdf25c to 672c149 Compare July 28, 2023 22:23
liuchengxu
liuchengxu previously approved these changes Jul 31, 2023
Copy link
Member

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

Some nits

crates/pallet-domains/src/lib.rs Show resolved Hide resolved
crates/pallet-domains/src/staking_epoch.rs Show resolved Hide resolved
vedhavyas
vedhavyas previously approved these changes Jul 31, 2023
@NingLin-P NingLin-P enabled auto-merge July 31, 2023 10:09
@NingLin-P NingLin-P disabled auto-merge July 31, 2023 11:12
@NingLin-P NingLin-P enabled auto-merge July 31, 2023 11:13
@NingLin-P NingLin-P force-pushed the domain-instance-fix branch from 077fe91 to ad52f72 Compare July 31, 2023 12:44
@NingLin-P NingLin-P dismissed stale reviews from liuchengxu and vedhavyas via 8e373c4 July 31, 2023 18:32
@NingLin-P NingLin-P force-pushed the domain-instance-fix branch from 5a15060 to 8e373c4 Compare July 31, 2023 18:32
@NingLin-P NingLin-P merged commit a401429 into main Aug 1, 2023
@NingLin-P NingLin-P deleted the domain-instance-fix branch August 1, 2023 02:30
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

Successfully merging this pull request may close these issues.

3 participants