Skip to content
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

code: Cleanup various proposal-related types #711

Open
romac opened this issue Dec 19, 2024 · 2 comments
Open

code: Cleanup various proposal-related types #711

romac opened this issue Dec 19, 2024 · 2 comments
Labels
code Code/implementation related P: high Priority: high
Milestone

Comments

@romac
Copy link
Member

romac commented Dec 19, 2024

We have somehow ended up with several different types with similar names holding similar but not identical information, which is quite confusing for everybody (me included).

Let's review those and see if we can deduplicate them or find better names for each of them:

pub struct ValueToPropose<Ctx: Context> {
pub height: Ctx::Height,
pub round: Round,
pub valid_round: Round,
pub value: Ctx::Value,
pub extension: Option<SignedExtension<Ctx>>,
}

pub struct ProposedValue<Ctx: Context> {
pub height: Ctx::Height,
pub round: Round,
pub valid_round: Round,
pub proposer: Ctx::Address,
pub value: Ctx::Value,
pub validity: Validity,
pub extension: Option<SignedExtension<Ctx>>,
}

pub struct LocallyProposedValue<Ctx: Context> {
pub height: Ctx::Height,
pub round: Round,
pub value: Ctx::Value,
pub extension: Option<SignedExtension<Ctx>>,
}

There is also the Proposal trait, which is not actually implemented for any of those, but only for the concrete proposal type, which does not help with the confusion:

pub trait Proposal<Ctx>
where
Self: Clone + Debug + Eq + Send + Sync + 'static,
Ctx: Context,
{
/// The height for which the proposal is for.
fn height(&self) -> Ctx::Height;
/// The round for which the proposal is for.
fn round(&self) -> Round;
/// The value that is proposed.
fn value(&self) -> &Ctx::Value;
/// The value that is proposed.
fn take_value(self) -> Ctx::Value;
/// The POL round for which the proposal is for.
fn pol_round(&self) -> Round;
/// Address of the validator who issued this proposal
fn validator_address(&self) -> &Ctx::Address;
}

@romac romac added the code Code/implementation related label Dec 19, 2024
@romac romac added this to the Phase 5 milestone Dec 19, 2024
@romac romac added the P: high Priority: high label Dec 19, 2024
@cason
Copy link
Contributor

cason commented Jan 6, 2025

From the consensus point of view, we have a Value that is anything that is proposed and decided in consensus. I am not sure whether the extensions in the code are decided or just data exchanged "outside consensus" (not committed).

A Proposal is a Value with the metadata needed to the first round step of Tendermint. Namely, height, round, valid round, and some identification of the proposer process (can be just a signature).

@cason
Copy link
Contributor

cason commented Jan 6, 2025

I see some relation with #365.

It is not 100% clear to me whether the Proposal from the Tendermint protocol carries a "full" Value or an unique identifier of the proposed "full" Value. On Quint this is trivial, as id(v) = v, but implementation-wise this is super relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code/implementation related P: high Priority: high
Projects
None yet
Development

No branches or pull requests

2 participants