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

687 populate grandpa and babe states from runtime #691

Conversation

hMitov
Copy link
Collaborator

@hMitov hMitov commented Jan 16, 2025

Description

feat: Add ServiceConsensusState interface with populateFromRuntime() method

  • Introduced ServiceConsensusState interface to define the initializeFromRuntime() method for RoundState and EpochState as requiring runtime data.
  • Implemented populateFromRuntime()
  • Integrated these state initializations within FullSyncMachine to ensure proper synchronization during node startup.
    Fixes Add functionality to populate grandpa and babe states from runtime. #687

Hristiyan Mitov added 3 commits January 16, 2025 14:07
…) method

- Introduced `ServiceConsensusState` interface to define the initializeFromRuntime() method for RoundState and EpochState as requiring runtime data.
- Implemented initializeFromRuntime()
- Integrated these state initializations within FullSyncMachine to ensure proper synchronization during node startup.
@hMitov hMitov marked this pull request as ready for review January 16, 2025 14:28
@hMitov hMitov self-assigned this Jan 16, 2025
@hMitov hMitov merged commit d93be7a into 666-refactor-services-states Jan 16, 2025
3 checks passed
@hMitov hMitov deleted the 687-populate-grandpa-and-babe-states-from-runtime branch January 16, 2025 15:34
return trieAccessor
.findStorageValue(Nibbles.fromBytes(":code".getBytes()))
.map(wasm -> buildRuntime(wasm, trieAccessor))
.orElseThrow(() -> new RuntimeException("Runtime code not found in the trie"));
Copy link
Member

Choose a reason for hiding this comment

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

Change this to one of our custom exceptions or create 1 if there isn't a good match. We want to generally avoid using generic exceptions such as this one.

@@ -58,4 +59,7 @@ public interface Runtime {
void persistsChanges();

void close();

List<Authority> getGrandpaApiGrandpaAuthorities();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is the name of the API method, but I'd go for something like getGrandpaApiAuthorities or getGrandpaAuthorities. Preferably the first one so it's similar to the babe API one.


@Override
public List<Authority> getGrandpaApiGrandpaAuthorities() {
return ScaleUtils.Decode.decode(call(RuntimeEndpoint.GRANDPA_API_GRANDPA_AUTHORITIES), new ListReader<>(new AuthorityReader()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Split this into two lines as it is longer than the suggested by Intellij.

public void populateDataFromRuntime(Runtime runtime) {
this.authorities = runtime.getGrandpaApiGrandpaAuthorities();
this.setId = BigInteger.ZERO;
this.roundNumber = BigInteger.ZERO;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As per spec the round at initialization should be 1.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not strictly related to this PR, but we need something like a base state to hold all the states. This way, we can have a centralized place to keep them and avoid injecting them as beans all over the place. The constructors are getting ridiculously large.

@@ -67,6 +68,8 @@ public class FullSyncMachine {
private final TrieStorage trieStorage = AppBean.getBean(TrieStorage.class);
private final RuntimeBuilder runtimeBuilder = AppBean.getBean(RuntimeBuilder.class);
private final EpochState epochState = AppBean.getBean(EpochState.class);
private final RoundState roundState = AppBean.getBean(RoundState.class);
private final GenesisBlockHash genesisBlockHash = AppBean.getBean(GenesisBlockHash.class);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need the GenesisBlockHash here. We have SyncState which holds the last finalized block number. We can compare it with 0 (genesis block number) and avoid this bean.

Zurcusa added a commit that referenced this pull request Jan 27, 2025
# Description
See the following PRs:
#688
#701
#691

---------

Co-authored-by: Hristiyan Mitov <[email protected]>
Co-authored-by: Hristiyan Mitov <[email protected]>
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.

Add functionality to populate grandpa and babe states from runtime.
3 participants