Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@svyatonik
Copy link
Contributor

I've been sure that BABE' light client is broken since #3583 , because it has been removed from tests there + I've seen no consensus cache usage in the code. This was until this morning, when I've noticed that the full client now also doesn't call runtime for anything else than configuration at genesis block. Since this (runtime calls) was the only reason that light client was requiring additional care (storing epoch data in the consensus cache), there's no other reasons why light client should not work.

So I'm restoring integration tests for LC in this PR.

@svyatonik svyatonik added the A0-please_review Pull request needs code review. label Dec 12, 2019
} else {
NUM_FULL_NODES
};
let expected_light_connections = NUM_FULL_NODES;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be NUM_LIGHT_NODES?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a number of connections at light nodes. Light nodes only connect to full nodes => NUM_FULL_NODES.

Copy link
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

I'm not entirely clear about the context behind the removal and now restoration of the LC integration tests, so maybe somebody else should chime in with regards to that. Code itself looks fine though.

@gavofyork gavofyork added A7-looksgoodcantmerge and removed A0-please_review Pull request needs code review. labels Dec 17, 2019
@bkchr bkchr merged commit 0102348 into master Dec 17, 2019
@bkchr bkchr deleted the recover_light_integration_tests branch December 17, 2019 07:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants