-
Notifications
You must be signed in to change notification settings - Fork 69
feat: initialize lean node architecture #679
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
Conversation
…ve dependency cycle issue
677229d to
32b3fcd
Compare
| // TODO: We need to replace this after PQC integration. | ||
| // For now, we only need ID for keystore. | ||
| pub struct LeanKeystore { | ||
| id: u64, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be integrated with our pqc crate after #668 merged.
| pub async fn start(self) { | ||
| info!("Lean Chain Service started"); | ||
|
|
||
| // TODO: Duplicate clock logic from ValidatorService. May need to refactor later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might introduce some kind of ClockService that emits tick every 12 / 4 seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were using tokio::time::interval in the ValidatorService. I can implement it once this PR is merged.
true, only
But maybe I can just implement |
|
96af999 deletes the original code. You might refer to the post where these methods are located in this version. |
unnawut
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Just 2 minor nits and 1 note so I'll approve in advance
|
It would be best if this PR gets at least 3 approval, so please don't hesitate to submit reviews! Thanks. |
ch4r10t33r
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Kayden-ML
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
What was wrong?
We need some base setup to accelerate our devs. I hope that this PR works like a cornerstone for our first lean node.
How was it fixed?
I wrote a post for providing more context and rationale on it. Reading the post will be helpful to understand the changes that are introduced in this PR.
Notes
About the original code
Most of the code is from the last PR(#672) (kudos to @unnawut). Refer to the part of the write-up for where they are originated from.
Running example
You don't have to set up
GENESIS_TIMEmanually: I put some hacky code for making our lives easier to review this PR. We might need some shell script or config to handle it later on.Also you don't have to set up validators, the code will put placeholders for missing validators.
^ You can see the chain is getting into consensus.
Feedbacks are welcomed
I think the design decision needs more feedbacks because it may have a critical flaws. I don't want to force this PR getting merged right now: I'm happy if we have some better consensus on our lean node architecture. Please share your opinions!
Next TODOs
NetworkService. I guess the best starting point is to consume a static peer list from the command line option.To-Do