Consensus Integration #1467
Consensus Integration #1467
Conversation
asonnino
added
Type: Major Feature
asonnino
added
Status: Blocked
asonnino
added
Status: Needs Review
Fixed |
|
This is built on top of your other PR correct? Lets land that one and then I'll be able to do a quicker/easier review of this one |
Previous Pr landed @bmwill |
| @@ -40,7 +40,7 @@ const MAX_DELAY_MILLIS: u64 = 5_000; // 5 sec | |||
|
|
|||
| pub struct AuthorityServer { | |||
| server: NetworkServer, | |||
| pub state: AuthorityState, | |||
| pub state: Arc<AuthorityState>, | |||
There was a problem hiding this comment.
We have a couple of fields within AuthorityState defined as Arc. Are they redundant with this, or do these two Arc serve different purposes?
There was a problem hiding this comment.
Good point looking into it. The one over the storage serves a different purpose, but I will double check the others.
There was a problem hiding this comment.
We can probably remove some but it will touch a lot of files. So I made an issue here #1496 that I can handle separately
| } | ||
|
|
||
| async fn load_execution_indices(&self) -> Result<ExecutionIndices, Self::Error> { | ||
| self._database.last_consensus_index() |
There was a problem hiding this comment.
can we rename _database to database since its used now?
There was a problem hiding this comment.
That is also an easy fix but it seems that many currently opened PR will have to rebase if I do it now. So I made an issue that I will quickly fix separately #1497
Putting all the pieces together to support shared-objects!