spec:ABCI2.0 - Uncommenting content related to vote xtensions and finalize block #421
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Co-authored-by: Lasaro <lasaro@informal.systems>
spec/abci/abci++_app_requirements.md
Outdated
| The sequence of `DeliverTx` calls is asynchronous but all those calls are enclosed by calls to `BeginBlock` and `EndBlock` which are synchronous. | ||
| --> | ||
|
|
||
| The Application must remember the latest height from which it |
There was a problem hiding this comment.
This could be moved to the next sub-section, right?
| @@ -286,18 +279,18 @@ In this case, potentially many proposed blocks will be disclosed to the Applicat | |||
| By the nature of Tendermint consensus algorithm, currently adopted in CometBFT, the number of proposed blocks received by the Application | |||
| for a particular height cannot be bound, so Application developers must act with care and use mechanisms | |||
| to bound memory usage. As a general rule, the Application should be ready to discard candidate states | |||
| before block execution, even if one of them might end up corresponding to the | |||
| decided block and thus have to be reexecuted upon `BeginBlock-DeliverTx-EndBlock`. | |||
| before `FinalizeBlock`, even if one of them might end up corresponding to the | |||
There was a problem hiding this comment.
This sentence is a little confusing.
spec/abci/abci++_app_requirements.md
Outdated
| decided block's data to the Application, which uses it to transition its state. | ||
|
|
||
| <!-- |
There was a problem hiding this comment.
We maybe state that this call is synchronous, isn't it?
| @@ -452,7 +362,11 @@ title: Methods | |||
| that the `RequestPrepareProposal.max_tx_bytes` limit is respected by those transactions | |||
| returned in `ResponsePrepareProposal.txs` . | |||
| * As a result of executing the prepared proposal, the Application may produce block events or transaction events. | |||
| The Application must keep those events until a block is decided. It will then forward the events to the `BeginBlock-DeliverTx-EndBlock` functions depending on where each event should be placed, thereby returning the events to CometBFT. | |||
| The Application must keep those events until a block is decided and then pass them on to CometBFT via | |||
There was a problem hiding this comment.
This here makes GitHub to format the remaining of this item list as a code. The comments, in particular, are rendered :D
| consensus execution: (a) at the moment a new proposal is to be created and (b) at the moment a | ||
| proposal is to be validated. The new interface allows block proposers to perform application-dependent | ||
| proposal is to be validated, and (c) at the moment a (precommit) vote is sent/received. |
There was a problem hiding this comment.
This could be a numbered list, right?
I would try to avoid embarking in refactoring of the text structure at this point (this is supposed to be a backport)
| Note that violation of Requirement 1 may just lead to further rounds, but will not compromise correctness. | ||
|
|
||
| Requirement 1 is particularly important if *p*'s Application fully executes prepared blocks in `PrepareProposal` | ||
| as a form of optimistic execution. |
There was a problem hiding this comment.
| as a form of optimistic execution. | |
| as a form of immediate execution. |
There was a problem hiding this comment.
I believe that optimistic is more accurate here, because it signals that the execution may be wasted, while immediate does not. It is also a term commonly used for similar execution of transactions in DBs. I understand that we have used immediate in some other places so far, though, so would need to review them.
There was a problem hiding this comment.
I tend to agree with Lasaro, immediate sounds like the execution is final and the state change is persisted.
|
Thank you @sergio-mena , @cason and @lasarojc for reviewing. All the comments that were not applied (most of all Daniel's suggestions for text reformatting and the discussion on optimistic vs. immediate execution, I merged in a doc so we can get back to this later) but I will merge this as to unblock other minor PRs regarding the spec. |
* Applied last comments from PR #421 * Spelling errors
The PR will be a draft until PR #407 is merged. After that it will be rebased to
feature/abci++vef.Closes #403 .