feat(code): Add actor integrated application that uses proposal only#1066
feat(code): Add actor integrated application that uses proposal only#1066ancazamfir wants to merge 29 commits intomainfrom
Conversation
…/malachite into anca/actor-proposal-only
| } | ||
|
|
||
| message SyncResponse { | ||
| ValueResponse value_response = 1; |
There was a problem hiding this comment.
This field should probably be explicitly optional and handled as such, cf. https://github.com/informalsystems/malachite/issues/1074#issuecomment-2939030159
…/malachite into anca/actor-proposal-only
|
Moved the application to https://github.com/informalsystems/malachite-apps/pull/2 |
romac
force-pushed
the
anca/actor-proposal-only
branch
from
00456e7 to
4b3ae5b
Compare
This reverts commit 4b3ae5b.
|
Re-instated the app code so that Nenad can post his pending comments and push a few commits |
nenadmilosevic95
left a comment
nenadmilosevic95
left a comment
There was a problem hiding this comment.
Some minor comments added
| pub const fn as_u64(&self) -> u64 { | ||
| self.0 | ||
| } | ||
|
|
||
| pub fn increment(&self) -> Self { | ||
| Self(self.0 + 1) | ||
| } | ||
|
|
||
| pub fn decrement(&self) -> Option<Self> { | ||
| self.0.checked_sub(1).map(Self) | ||
| } |
There was a problem hiding this comment.
Should we put this inside impl malachitebft_core_types::Height for Height block ?
There was a problem hiding this comment.
Yes we could. Maybe we open an issue to discuss and solve across all apps. If we move all methods out of the concrete implementation then the app code will have to do calls like:
use malachitebft_core_types::CommitCertificate, Height as HeightTrait;
let start_height = HeightTrait::increment(&latest_block_height);
btw, the Height trait implements increment() and decrement() already.
There was a problem hiding this comment.
I made some changes in db158ed so you see how it would look.
| pub fn id(&self) -> ValueId { | ||
| ValueId::new(self.value.block_hash.clone()) | ||
| } |
There was a problem hiding this comment.
Why do we define this method two times?
|
|
||
| state.consensus = Some(consensus.clone()); | ||
|
|
||
| tokio::time::sleep(Duration::from_millis(200)).await; |
There was a problem hiding this comment.
why this sleep here?
There was a problem hiding this comment.
is see it's removed. We do this sleep in all apps, @romac mentioned something around connection errors. We should probably investigate and clean the others if not required.
|
|
||
| // If we have already built or seen one or more values for this height and round, | ||
| // feed them back to consensus. This may happen when we are restarting after a crash. | ||
| replay_undecided_values(state, height, round).await?; |
There was a problem hiding this comment.
What are we replaying here?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
To discuss sync:
I'll schedule a sync soon. |
| .. | ||
| } => on_decided(state, &consensus, certificate, &self.metrics).await, | ||
|
|
||
| HostMsg::GetDecidedValue { height, reply_to } => { |
There was a problem hiding this comment.
5 participants
WIP
Closes: #XXX
Helper app for #953
Not completely Part agnostic because:
ProposalParttypeProtobufCodec: Codec<types::proposal::ProposalPart>PR author checklist
For all contributors
RELEASE_NOTES.mdif the change warrants itBREAKING_CHANGES.mdif the change warrants itFor external contributors