Skip to content

[consensus/simplex] Review Relay::broadcast Usage #2394

@patrick-ogrady

Description

@patrick-ogrady

If the Automaton proposes before the timeout, we invoke Relay::broadcast before issuing our own vote:

// Construct proposal
let proposal = Proposal::new(
context.round,
context.parent.0,
proposed,
);
if !self.state.proposed(proposal) {
warn!(round = ?context.round, "dropped our proposal");
continue;
}
view = self.state.current_view();
// Notify application of proposal
self.relay.broadcast(proposed).await;

In #2393, we persist the block in marshal at this point to ensure that if our node dies between this broadcast and notarization we have the block we built.

While "correct", this is very very nuanced (and error-prone). Why should broadcast be expected to persist anything? If this order is changed in simplex (as the average person would not assume broadcast == "store this"), we could see a concerning regression.

We should review whether or not this is the right approach (or if we should just assume that a block should be stored before returning from propose like we do for verify:

if application_valid {
marshal.verified(context.round, block).await;
}

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

Status

In Progress

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions