docs: ADR-064 ABCI++ (2.0) #14657

alexanderbez · 2023-01-17T19:20:05Z

Description

This ADR outlines the design of ABCI++ (2.0), which include VoteExtensions and FinalizeBlock, in the Cosmos SDK.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

alexanderbez · 2023-01-23T20:52:23Z

So prior to documenting anything for vote extensions, I figured it would be best to discuss various approaches more broadly first, since vote extensions are the most interesting surface area of ABCI 2.0 and we should nail the API for best dev UX.

What I currently have in mind is something of the following:

Allow app devs to set ExtendVoteHandler(sdk.Context, abci.RequestExtendVote) abci.ResponseExtendVote
Allow app devs to set VerifyVoteExtensionHandler(sdk.Context, abci.RequestVerifyVoteExtension) abci.ResponseVerifyVoteExtension

These ^ should be pretty much be a given and not contentious.

What the more interesting and trickier part will be is how to handle the state of vote extensions and ensuring state ends up where and when it needs to. Specifically, I'm thinking:

Vote extensions can be thought of as short-lived ephemeral state which is scoped to the execution of a single proposal (block) and made available only to the proposer at the next height

We define some VoteExtState that can loosely be defined as follows:

  type VoteExtState struct {
  	Hash       []byte                      // current proposal's block hash, set in ProcessProposal
  	Height     uint64                      // current proposal's block height, set in ProcessProposal
  	Local      VoteExt                     // the local process' vote extension
  	Validators map[sdk.ConsAddress]VoteExt // the vote extensions received from validators during VerifyVoteExtension
  }

  type VoteExt struct {
  	Hash    []byte
  	Height  uint64
  	VoteExt []byte
  	Status  abci.VerifyStatus
  }

The VoteExtState is (re)set on each ProcessProposal call (maybe keeping them for the past 2 heights?)
A local validator process creates a vote extension and BaseApp populates Local
When executing VerifyVoteExtension and thus evaluating vote extensions of a subset of other validators, a validator process can use the committed blockchain state in conjunction with the ephemeral VoteExtState, which we'll pass to VerifyVoteExtensionHandler

Assuming a proposal is accepted, the vote extensions for that block will be made available to the proposer only at the next block, during PrepareProposal. Now these vote extensions can be useful to other validators during ProcessProposal of that same height.

So here is another part we need to solve. How do we make this available? One thing I can think of is encourage apps to insert a special vote extensions tx into the proposal during PrepareProposal.

ttl33 · 2023-01-26T02:10:37Z

Before jumping into the details of what changes are needed for CosmosSDK to accommodate VoteExtensions, I wanted to clarify if my understanding of how VoteExtensions work across Tendermint and CosmosSDK on the right track. Here's a layout of what I think is happening:

Height X

Validator collects pre-commit votes along w VoteExtensions. This process is already handled by Tendermint. The part where CosmosSDK app influences is only the VerifyVoteExtensions part.
- Q1: CosmosSDK exposes SetVerifyVoteExtensionHandler to the app for customization and then Tendermint calls this VerifyVoteExtensionHandler when collecting pre-commit votes along w VoteExtensions. Is this correct?
- Q2: There’s no extra bookkeeping that CosmosSDK needs to do, because this is all handled on the Tendermint side. Is this correct?

Height X+1

During PrepareProposal, Tendermint gives VoteExtension data from Height X to the app. Then the app has the ability to use VoteExtension data to derive some new data (i.e. if VoteExtension data contains oracle prices from each validator, then PrepareProposal can get the median price and add it to the proposal).
- Q3: It’s unclear if the CosmosSDK app has to do anything to put the VoteExtension data on chain (i.e. part of block for height X+1). For example, does the app need to process + return the VoteExtensions in the Response for the VoteExtension data to be persisted?
  - If the app has the power to process + return the VoteExtensions then ProcessProposal will prob want to verify the VoteExtension data again to make sure they are untampered. This also opens up the possibility that a misbehaving proposer/validator can throw out certain VoteExtension data or replay old VoteExtension data. Something to think about 🤔
During ProcessProposal, validators would need access to the exact set of VoteExtension data that the proposing validator saw in order to validate the derived data. (i.e. is the median price correct based on the VoteExtensions)?
- Q4: How can ProcessProposal have access to the same set of VoteExtension data that the proposing validator saw? Is this handled by Tendermint?

If the above assumptions are true, I wonder what VoteExtState struct and VoteExt struct are used for. It’s unclear why the CosmosSDK app needs to keep track of the VoteExtension state.

Would you be able to double check my understanding above?

docs/architecture/adr-064-abci-2.0.md

Co-authored-by: ttl33 <[email protected]>

alexanderbez · 2023-01-26T15:00:29Z

@ttl33 happy to answer your Qs and thanks for taking a look!

Q1: CosmosSDK exposes SetVerifyVoteExtensionHandler to the app for customization and then Tendermint calls this VerifyVoteExtensionHandler when collecting pre-commit votes along w VoteExtensions. Is this correct?

It first calls ExtendVote -- this is the actual vote extension which is included and broadcasted along with the pre-commit. Then, a separate ABCI method, VerifyVoteExtension, is called, where it will verify the vote extension of that particular pre-commit. So they are two different ABCI methods called in sequence.

Q2: There’s no extra bookkeeping that CosmosSDK needs to do, because this is all handled on the Tendermint side. Is this correct?

What do you mean by bookkeeping exactly? If I understand you correctly, all the SDK is responsible for is (A) creating a vote extension, which can be non-deterministic btw, and (B) verifying other validators vote extensions.

Q3: It’s unclear if the CosmosSDK app has to do anything to put the VoteExtension data on chain (i.e. part of block for height X+1). For example, does the app need to process + return the VoteExtensions in the Response for the VoteExtension data to be persisted?

If the app has the power to process + return the VoteExtensions then ProcessProposal will prob want to verify the VoteExtension data again to make sure they are untampered. This also opens up the possibility that a misbehaving proposer/validator can throw out certain VoteExtension data or replay old VoteExtension data. Something to think about 🤔

Q4: How can ProcessProposal have access to the same set of VoteExtension data that the proposing validator saw? Is this handled by Tendermint?

I'll combine answers to Q3 and Q4 together. So as far as I understand, from Tendermint's PoV, vote extensions are solely used in the pre-commit voting phase and then included in RequestPrepareProposal for the next height. They are NOT included in the state machine implicitly. They may be included in the actual block header -- we need to verify this.

This ties into the crux of the problem! How do we propogate and provide the vote extensions along with any proposer-specific-derived-data to validators in ProcessProposal?

So my proposal is that apps include an SDK-provided special tx type called VoteExtsTx which includes all the vote extensions and any other validator-derived data. But as you've pointed out, there is nothing guaranteeing the proposer didn't modify any data (e.g. add, modify or removed vote exts).

ttl33 · 2023-01-26T19:52:21Z

Q1: CosmosSDK exposes SetVerifyVoteExtensionHandler to the app for customization and then Tendermint calls this VerifyVoteExtensionHandler when collecting pre-commit votes along w VoteExtensions. Is this correct?

It first calls ExtendVote -- this is the actual vote extension which is included and broadcasted along with the pre-commit. Then, a separate ABCI method, VerifyVoteExtension, is called, where it will verify the vote extension of that particular pre-commit. So they are two different ABCI methods called in sequence.

This matches my understanding!

Q2: There’s no extra bookkeeping that CosmosSDK needs to do, because this is all handled on the Tendermint side. Is this correct?

What do you mean by bookkeeping exactly? If I understand you correctly, all the SDK is responsible for is (A) creating a vote extension, which can be non-deterministic btw, and (B) verifying other validators vote extensions.

What I meant here is: Other than providing hooks to call ExtendVote and VerifyVoteExtension, CosmosSDK does not have to keep track of what the content of ExtendVote and what the result of VerifyVoteExtension were.

Q3 + Q4

vote extensions are solely used in the pre-commit voting phase and then included in RequestPrepareProposal for the next height. They are NOT included in the state machine implicitly. They may be included in the actual block header -- we need to verify this.

I think this info is critical in determining if the proposing validator needs to explicitly put the VoteExtension data (that's included in RequestPrepareProposal) in the proposal to persist it on-chain or not. My assumption here is that the app should have an explicit control over what VoteExtension data to put on-chain. There are two main use-cases that I am thinking of:

Oracle Prices: proposer would want to put all the VoteExtension data so that other validators can verify that the proposed price is indeed the median price of all the votes.
Threshold Crypto: proposer does NOT want to put all VoteExtension data on-chain. So it will opt to put some subset (including "no data") or some derived data on-chain.

In example 1, it's desired that the proposer puts all VoteExtension data to ProcessProposal for validation untampered. However, in example 2, it's undesirable to do so. So I think giving the CosmosSDK app the handle this makes sense to me and gives the app more flexibility.

So my proposal is that apps include an SDK-provided special tx type called VoteExtsTx which includes all the vote extensions and any other validator-derived data.

I think I have similar thoughts. More concretely, something like:

RequestPrepareProposal {
    ... 
    Txs [][]byte
    ExtendVotes [][]byte // or some other data type that's more strictly typed
}

ResponsePrepareProposal {
   ...
   Txs [][]byte // app can inject some info here that's derived from `ExtendVotes`
   ExtendVotes [][]byte // app has the ability to filter which `ExtendVotes` to include and not
}

Note: CosmosSDK can package `Txs` and `ExtendVotes` in `ResponsePrepareProposal` by append 
them before sending them back to Tendermint or can serialize them using something like 
`serialize(`{Txs: [][]byte, ExtendVotes: [][]byte}`)

RequestProcessProposal {
   ...
   Txs [][]byte
   ExtendVotes [][]byte // this is essentially what was included in `ResponsePrepareProposal`
}

alexanderbez · 2023-01-26T20:41:39Z

What I meant here is: Other than providing hooks to call ExtendVote and VerifyVoteExtension, CosmosSDK does not have to keep track of what the content of ExtendVote and what the result of VerifyVoteExtension were.

It does not have to correct, but I think to fully make VerifyVoteExtensions useful, it should keep track of them in-memory. E.g. consider the case when VerifyVoteExtensions verifies prices are within some mean or distance of other prices it receives. It may want to reject ones that are too far out. In order to do this, it would need to keep track. That's why I proposed to keep some ephemeral state for vote extensions when VerifyVoteExtensions is called.

I think this info is critical in determining if the proposing validator needs to explicitly put the VoteExtension data (that's included in RequestPrepareProposal) in the proposal to persist it on-chain or not. My assumption here is that the app should have an explicit control over what VoteExtension data to put on-chain.

Vote extensions do not exist on-chain, at least not in the app-side. I don't see any way of doing this unless you send them along in special txs like I proposed. Besides, processProposalState is ephemeral and never committed.

I think I have similar thoughts. More concretely, something like: ...

What you're describing is a fundamental change to the Tendermint ABCI++ protocol, which is outside our domain. IIRC, @sergio-mena stated that they discussed various approaches and ultimately decided on not including ExtendVotes in RequestProcessProposal because it was more flexible.

I agree that if RequestProcessProposal included the vote extensions, it would make this entire thing dead simple. But it doesn't.

tac0turtle · 2023-01-26T21:31:17Z

When i think of the of voteextensions and how it could be used i could see this:

Allow app devs to set ExtendVoteHandler(sdk.Context, abci.RequestExtendVote) abci.ResponseExtendVote

Allow app devs to set VerifyVoteExtensionHandler(sdk.Context, abci.RequestVerifyVoteExtension) abci.ResponseVerifyVoteExtension

Being an interface that a module implements if they'd like access to the feature. It would be similar to the begin/end block interfaces and be optional to applications. The handler in baseapp can be overridden but in the world of cosmwasm where a contract wants to take advantage of this, it would need to be in the module instead of in baseapp.

Was that what you were mentioning?

ttl33 · 2023-01-26T22:04:27Z

It does not have to correct, but I think to fully make VerifyVoteExtensions useful, it should keep track of them in-memory. E.g. consider the case when VerifyVoteExtensions verifies prices are within some mean or distance of other prices it receives. It may want to reject ones that are too far out. In order to do this, it would need to keep track. That's why I proposed to keep some ephemeral state for vote extensions when VerifyVoteExtensions is called.

I see. I think the source of confusion was the fact that I thought VoteExtState was meant for Height X+1, but you meant VoteExtState to be something be part of the Height X (in above comment).

In this case, I don't necessarily think the CosmosSDK needs to prescribe a bookkeeping mechanism (i.e. VoteExtState) to keep track of what votes each validator has seen and what the Verify results were. If the app needs to hold onto incoming vote extension data and their results, they can easily add something to VerifyVoteExtensions to pipe the data to some temporary datastore of their choice.

IMO, your suggestion with VoteExtState could make sense for certain situations. So, a reasonable approach would be to provide some default behaviors like VoteExtState but give the app the flexibility to change it. Essentially, similar to how CosmosSDK approached the app-side mempool (code), where some example app-side mempool impl is there, but app can impl their own or opt out.

Vote extensions do not exist on-chain, at least not in the app-side. I don't see any way of doing this unless you send them along in special txs like I proposed. Besides, processProposalState is ephemeral and never committed.

I agree. PrepareProposal should return some list of VoteExtension data in the response that should be included on-chain.

What you're describing is a fundamental change to the Tendermint ABCI++ protocol, which is outside our domain. IIRC, @sergio-mena stated that they discussed various approaches and ultimately decided on not including ExtendVotes in RequestProcessProposal because it was more flexible.

Thinking this over, it makes sense to not include the VoteExtension data in ProcessProposal because apps don't necessarily need to make consensus decisions on the VoteExtension data that can be non-deterministic. Then, would it make sense to include the VoteExtension data as part of FinalizeBlock?

I am curious how the final VoteExtension data that the proposer decided to put would be validated on other validators? Would there be some "validation" step for the VoteExtension data in FinalizeBlock step?

alexanderbez · 2023-01-27T15:36:20Z

Yes @tac0turtle but that part of the ADR is trivial, the crux of what's unsolved is how to propagate committed vote extensions to validators in ProcessProposal.

@ttl33

I see. I think the source of confusion was the fact that I thought VoteExtState was meant for Height X+1, but you meant VoteExtState to be something be part of the Height X (in #14657 (comment)).

Yup! Since at X+1 they're already verified and committed.

In this case, I don't necessarily think the CosmosSDK needs to prescribe a bookkeeping mechanism (i.e. VoteExtState) to keep track of what votes each validator has seen and what the Verify results were. If the app needs to hold onto incoming vote extension data and their results, they can easily add something to VerifyVoteExtensions to pipe the data to some temporary datastore of their choice.

Why? Why bring in dependencies and complexities when you can just easily keep track of it in memory? You don't need to persist anything.

I am curious how the final VoteExtension data that the proposer decided to put would be validated on other validators? Would there be some "validation" step for the VoteExtension data in FinalizeBlock step?

Yes -- this is the tricky part and what we need to discuss and figure out.

Essentially, this entire thing boils down to 2 questions:

How do we propagate vote extensions to validators in ProcessProposal?
How do we ensure the validators can verify the propagated vote extensions in ProcessProposal

ttl33 · 2023-01-27T20:05:31Z

Why? Why bring in dependencies and complexities when you can just easily keep track of it in memory? You don't need to persist anything.

Exactly. I am advocating for giving the app the flexibility to handle the the VerifyVoteExtension results as they choose. For example, if CosmosSDK always keeps track of each VoteExtension data and the results in-memory and the app has no use for it, then I think it would waste memory space. So I don't think it's necessary for the CosmosSDK to bake this behavior into the framework.

How do we propagate vote extensions to validators in ProcessProposal? How do we ensure the validators can verify the propagated vote extensions in ProcessProposal?

Hmm, from our convo earlier, I was under the impression that the vote extension won't be part of ProcessProposal.

For oracle prices, it's critical that there's some guarantee or validation process on the VoteExtension data is indeed from 2/3+ validators with valid signatures. But not sure if other VoteExtension usages need this guarantee. WDYT?

BrendanChou · 2023-01-28T00:47:51Z

Essentially, this entire thing boils down to 2 questions:

How do we propagate vote extensions to validators in ProcessProposal?

How do we ensure the validators can verify the propagated vote extensions in ProcessProposal

I agree with this assessment and it's a good way of framing the discussion points.

For question number 2, I feel it will be trickier. In addition, I'm worried that it will be extra hard to verify this information if the validator set changes in any way since the previous block.

alexanderbez · 2023-02-01T01:10:15Z

if CosmosSDK always keeps track of each VoteExtension data and the results in-memory and the app has no use for it,

What do you mean? It absolutely does have use for it. During height H, the app's VerifyVoteExtensionHandler will use this ephemeral state to perform vote extension verification for all vote extensions is receives during H. How and what it uses is entirely dependent on the app.

From what I gather, we have a firm grasp on what vote extensions are. Yet, we do not have a firm grasp on how they're meant to be used, propagated effectively and validated safely. @thanethomson is scheduling a sync call with a stakeholders so we can all better understand the complete picture. Once this happens, I feel like we can come up with a good solution 😄

ttl33

Overall LGTM. Thanks for putting this together Bez!

Left some non-blocking comments and nits

ttl33 · 2023-03-23T22:16:42Z

docs/architecture/adr-064-abci-2.0.md

+
+### `VoteExtensions`
+
+Similarly for `PrepareProposal` and `ProcessProposal`, we propose to introduce


To clarify - are the following statements correct? If so, it would be nice to add some clarifying statements in the ADR

For each block round, every validator node will call ExtendVoteHandler

For each block round and for each vote they receive from other nodes, every validator node will call VerifyVoteExtensionHandler

So if there are X number of validators for a block height H round 1, at the network level, there will be X number of calls to 1 and up to (X-1) * X calls to 2?

Of course, happy to clarify!

Yes, for every round, CometBFT will execute an ExtendVote request. In fact, it does this for pretty much all ABCI methods.

Yes, this is also correct from my understanding. For each height and round within that height, CometBFT will execute a VerifyVoteExtension request.

Ideally, when you extend the vote and then verify all incoming vote exts, you receive 2/3+ valid ones and avoid having to proceed to the next round.

So if there are X number of validators for a block height H round 1, at the network level, there will be X number of calls to 1 and up to (X-1) * X calls to 2?

Yes, I believe there will be X calls to ExtendVote. For VerifyVoteExtension, the proposer just needs 2/3 pre-commits to be valid, so this number can vary, but in theory it can be up to X-1, yes.

Updated to include a statement on this. LMK if you think it suffices :)

sg, thanks for clarifying. feel free to resolve this comment

ttl33 · 2023-03-23T22:20:45Z

docs/architecture/adr-064-abci-2.0.md

+verification. This state management could be ephemeral or could be some form of
+on-disk persistence.
+
+Example:


Nice example.

Nit: Will there be helper methods available in CosmosSDK and examples implemented in the simapp?

I'm not sure if there will be any since it could be so application specific. What kind of example or basic implementation ideas did you have in mind?

I think the price example is a simple but sufficient one.

For helper methods, there could be

no-op ExtendVote/VerifyVoteHandlers.

encoding/decoding helper methods (?) if these can be generic?

setter (?) if these can be generic? i.e. you have SetPrices

ttl33 · 2023-03-23T22:23:43Z

docs/architecture/adr-064-abci-2.0.md

+// a vote extension, such as fetching a series of prices for supported assets.
+func (h VoteExtensionHandler) ExtendVoteHandler(ctx sdk.Context, req abci.RequestExtendVote) abci.ResponseExtendVote {
+	prices := GetPrices(ctx, h.mk.Assets())
+	bz, err := EncodePrices(h.cdc, prices)


For my own understanding - why does the application have to encode the data before setting the ResponseExtendVote.VoteExtension?

Is it possible that the VoteExtension data to be not encoded if the entire ResponseExtendVote will be encoded by the downstream?

ResponseExtendVote.VoteExtension is an opaque byte slice from CometBFT's perspective. We have to transform real/domain data to raw data somehow.

got it, thanks for clarifying. feel free to resolve this comment!

ttl33 · 2023-03-23T22:24:23Z

docs/architecture/adr-064-abci-2.0.md

+	// store our vote extension at the given height
+	//
+	// NOTE: Vote extensions can be overridden since we can timeout in a round.
+	SetPrices(h.state, req, bz)


Why is this needed?

This is just a demo/example. I did this to demonstrate setting/saving a node's local vote ext.

Why? Because when it comes down to verifying other validators vote exts, you might want to compare theirs vs yours. E.g. in the case of oracle prices, you might want to verify that any incoming vote ext is no more than X% +/- from yours.

Of course this is just an example, and you don't have to save or keep track of anything at all.

I see, this would be a good example to have

ttl33 · 2023-03-23T22:25:10Z

docs/architecture/adr-064-abci-2.0.md

+		return abci.ResponseVerifyVoteExtension{Status: REJECT}
+	}
+
+	if err := ValidatePrices(h.state, req, prices); err != nil {


To confirm, there's no need to verify the vote extension signature here right? I assume that will be already taken care of by the CometBFT before this data gets to this point

Correct. There is no need to verify signatures in VerifyVoteExtension.

got it, thanks for clarifying. it might be useful to leave a comment that there's no need to verify here due to the reason above

ttl33 · 2023-03-23T22:27:39Z

docs/architecture/adr-064-abci-2.0.md

+
+// ValidateVoteExtensions is a function that an application can execute in
+// ProcessProposal to verify vote extension signatures.
+func (app *BaseApp) ValidateVoteExtensions(ctx sdk.Context, currentHeight int64, extCommit abci.ExtendedCommitInfo) error {


To confirm - the signature verification logic stated here would be available as a helper method in CosmosSDK right? Trying to avoid the app needing to implement the same logic 😆

Correct, exactly!

got it, thanks for clarifying. feel free to resolve this comment!

docs/architecture/adr-064-abci-2.0.md

ttl33 · 2023-03-23T22:33:28Z

docs/architecture/adr-064-abci-2.0.md

+What this means is that any explicit state changes in a `ProcessProposal` handler
+using the injected `processProposalState.Context` will be written to state! An
+application must be careful to execute `ResetProcessProposalState` where and when
+appropriate.


To confirm my understanding, is the following correct?

ProcessProposalHandler: does a bunch of state modifications

ProcessProposalHandler: if ResetProcessProposalState is called, then all the modifications made in 1 is reverted. If not, then all the modifications will be passed onto the next step, which is FinalizeBlock which will update the actual state

Since SetVoteExtResult is called after the Reset, the VoteExtResult will be available in FinalizeBlock state?

This was the cleanest way I could think of doing it.

got it, thanks for clarifying. feel free to resolve this comment!

ttl33 · 2023-03-23T22:40:54Z

docs/architecture/adr-064-abci-2.0.md

+		// 
+		// NOTE: Applications must call this to ensure no state transactions up to now
+		// are committed!
+		h.app.ResetProcessProposalState()


What's the reasoning for adding ResetProcessProposalState?

Is it because we want to preserve the work done in ProcessProposal to FinalizeBlock in certain use cases?

The reason is that generally speaking, you don't want state mutations from ProcessProposal to be committed because you're just going to re-do those mutations during FinalizeBlock anyway. However, there are situations where you DO want SOME mutations to commit from ProcessProposal, those being anything to do with vote extensions.

Hence, the need or ability for some way to say "Hey, I want this to be committed but not this other stuff."

Co-authored-by: ttl33 <[email protected]>

tac0turtle

LGTM, nicely written adr

tac0turtle · 2023-03-28T16:58:43Z

docs/architecture/adr-064-abci-2.0.md

+  will add significant performance overhead to `ProcessProposal`. Granted, the
+	signature verification process can happen concurrently using an error group
+	with `GOMAXPROCS` goroutines.


Suggested change

will add significant performance overhead to `ProcessProposal`. Granted, the

signature verification process can happen concurrently using an error group

with `GOMAXPROCS` goroutines.

will add significant performance overhead to `ProcessProposal`. Granted, the

signature verification process can take advantage of multiple different optimisations (parallelisation, batching and/or aggregation).

minor nit

facundomedica

LGTM 🚀

sergio-mena

One general observation after reading this ADR is that there are no provisions for upgrading from v0.47.x (or earlier), which doesn't have vote extensions, to the new version, where vote extensions are mandatory.
At the CometBFT level, we have dealt with this both at the level of the spec and implementation. Please take a look, although the best source to get context on this is still CometBFT's RFC100.

sergio-mena · 2023-03-30T13:45:01Z

docs/architecture/adr-064-abci-2.0.md

+safety purposes, we also propose that the proposer itself verify all the vote
+extension signatures it receives in `RequestPrepareProposal`.


Real apps shouldn't need to do this, since all those extensions are supposed to have been validated upon reception in H-1. Not that, at this point, all this info is coming from the local CometBFT, not from the network.

sergio-mena · 2023-03-30T13:46:05Z

docs/architecture/adr-064-abci-2.0.md

+is decided.
+
+In other words, `FinalizeBlock` encapsulates the current ABCI execution flow of
+`BeginBlock`, one or more `DeliverTx`, and `EndBlock` into a single ABCI method.


's/one or more/zero or more/'

alexanderbez · 2023-04-01T01:02:50Z

One general observation after reading this ADR is that there are no provisions for upgrading from v0.47.x (or earlier), which doesn't have vote extensions, to the new version, where vote extensions are mandatory. At the CometBFT level, we have dealt with this both at the level of the spec and implementation. Please take a look, although the best source to get context on this is still CometBFT's RFC100.

@sergio-mena can you clarify why provisions need to be explicitly be made for upgrading from SDK v0.47.x to v0.48.x? The latter is not state compatible with the former, so a network upgrade will be needed. If an upgrade occurs at height H, there will be no vote extensions at H-1 anyway. So what explicit case(s) do we need to handle exactly?

adizere · 2023-04-20T16:40:39Z

So what explicit case(s) do we need to handle exactly?

I read through the RFC (this part seemed relevant) and I'm not sure either what's the case. Did you get an answer @alexanderbez ?

alexanderbez · 2023-04-20T19:16:14Z

Yeah this is addressed in the ADR 👍

sergio-mena · 2023-04-26T10:36:40Z

@alexanderbez thanks for addressing the upgrade path in #15721

updates

fecacb6

alexanderbez changed the title ~~docs: ADR-064 - ABCI++ (2.0)~~ docs: ADR-064 ABCI++ (2.0) Jan 17, 2023

github-actions bot added the Type: ADR label Jan 17, 2023

alexanderbez added T:Docs Changes and features related to documentation. C:ABCI T: ADR An issue or PR relating to an architectural decision record S:blocked Status: Blocked and removed Type: ADR S:blocked Status: Blocked labels Jan 17, 2023

updates

11a6e97

github-actions bot added the Type: ADR label Jan 19, 2023

alexanderbez added 5 commits January 19, 2023 15:47

updates

eadbe04

updates

356782d

updates

ea7ef35

updates

59203f4

updates

8d73043

updates

34c616f

ttl33 reviewed Jan 26, 2023

View reviewed changes

docs/architecture/adr-064-abci-2.0.md Outdated Show resolved Hide resolved

docs/architecture/adr-064-abci-2.0.md Outdated Show resolved Hide resolved

docs/architecture/adr-064-abci-2.0.md Show resolved Hide resolved

Update docs/architecture/adr-064-abci-2.0.md

73b0f58

Co-authored-by: ttl33 <[email protected]>

alexanderbez and others added 7 commits March 14, 2023 13:46

updates

a4ad8c8

updates

f9c1909

updates

28d3890

updates

4ba1fcd

updates

2d177bd

updates

2477333

Merge branch 'main' into bez/12272-abci-2.0-adr

22a7763

alexanderbez marked this pull request as ready for review March 20, 2023 19:35

alexanderbez requested a review from a team as a code owner March 20, 2023 19:35

alexanderbez added 2 commits March 21, 2023 10:24

Merge branch 'main' into bez/12272-abci-2.0-adr

dce3408

Merge branch 'main' into bez/12272-abci-2.0-adr

af6facc

ttl33 approved these changes Mar 23, 2023

View reviewed changes

ttl33 reviewed Mar 23, 2023

View reviewed changes

ttl33 approved these changes Mar 23, 2023

View reviewed changes

alexanderbez and others added 5 commits March 27, 2023 12:38

Merge branch 'main' into bez/12272-abci-2.0-adr

289d595

updates

223fa29

Update docs/architecture/adr-064-abci-2.0.md

a83608f

Co-authored-by: ttl33 <[email protected]>

Merge branch 'main' into bez/12272-abci-2.0-adr

7d07659

Merge branch 'main' into bez/12272-abci-2.0-adr

494592c

tac0turtle approved these changes Mar 28, 2023

View reviewed changes

facundomedica approved these changes Mar 28, 2023

View reviewed changes

Merge branch 'main' into bez/12272-abci-2.0-adr

8969f4c

alexanderbez enabled auto-merge (squash) March 29, 2023 00:17

alexanderbez merged commit bb86630 into main Mar 29, 2023

alexanderbez deleted the bez/12272-abci-2.0-adr branch March 29, 2023 00:20

sergio-mena reviewed Mar 30, 2023

View reviewed changes


		### `VoteExtensions`

		Similarly for `PrepareProposal` and `ProcessProposal`, we propose to introduce

		safety purposes, we also propose that the proposer itself verify all the vote
		extension signatures it receives in `RequestPrepareProposal`.

docs: ADR-064 ABCI++ (2.0) #14657

docs: ADR-064 ABCI++ (2.0) #14657

Conversation

alexanderbez commented Jan 17, 2023 • edited Loading

Description

Author Checklist

Reviewers Checklist

alexanderbez commented Jan 23, 2023 • edited Loading

ttl33 commented Jan 26, 2023

alexanderbez commented Jan 26, 2023

ttl33 commented Jan 26, 2023

alexanderbez commented Jan 26, 2023

tac0turtle commented Jan 26, 2023

ttl33 commented Jan 26, 2023

alexanderbez commented Jan 27, 2023 • edited Loading

ttl33 commented Jan 27, 2023

BrendanChou commented Jan 28, 2023

alexanderbez commented Feb 1, 2023 • edited Loading

ttl33 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttl33 Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderbez Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tac0turtle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facundomedica left a comment

Choose a reason for hiding this comment

sergio-mena left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderbez commented Apr 1, 2023

adizere commented Apr 20, 2023

alexanderbez commented Apr 20, 2023

sergio-mena commented Apr 26, 2023

alexanderbez commented Jan 17, 2023 •

edited

Loading

alexanderbez commented Jan 23, 2023 •

edited

Loading

alexanderbez commented Jan 27, 2023 •

edited

Loading

alexanderbez commented Feb 1, 2023 •

edited

Loading

ttl33 Mar 28, 2023 •

edited

Loading

alexanderbez Mar 28, 2023 •

edited

Loading