diff --git a/spec/abci++/abci++_app_requirements_002_draft.md b/spec/abci++/abci++_app_requirements_002_draft.md index 994e1999d..8ceabd9a4 100644 --- a/spec/abci++/abci++_app_requirements_002_draft.md +++ b/spec/abci++/abci++_app_requirements_002_draft.md @@ -37,33 +37,42 @@ _AppHash_, _TxResults_, _ConsensusParams_, and _ValidatorUpdates_, or (b) log an error if the Application is in next-block execution mode and _does_ provide values for _AppHash_, _TxResults_, _ConsensusParams_, or _ValidatorUpdates_ (the values provided will be ignored). -* Requirement 3 [`PrepareProposal`, `ProcessProposal`, coherence]: For any two correct processes $p$ and $q$, +* Requirement 3 [`PrepareProposal`, timeliness] If $p$'s Application fully executes prepared blocks in + `PrepareProposal` and the network is in a synchronous period while processes $p$ and $q$ are in $r_p$, + then the value of _TimeoutPropose_ at $q$ must be such that $q$ does not prevote _nil_ in $r_p$ due to + $q$'s propose timer timing out. + +Full execution of blocks at `PrepareProposal` time stands on Tendermint's critical path. Thus, +Requirement 3 ensures the Application will set a value for _TimeoutPropose_ such that the time it takes +to fully execute blocks in `PrepareProposal` does not interfere with Tendermint's propose timer. + +* Requirement 4 [`PrepareProposal`, `ProcessProposal`, coherence]: For any two correct processes $p$ and $q$, if $q$'s Tendermint calls `RequestProcessProposal` on $v'_p$, $q$'s Application returns Accept in `ResponseProcessProposal`. -Requirement 3 makes sure that blocks proposed by correct processes _always_ pass the correct receiving process's +Requirement 4 makes sure that blocks proposed by correct processes _always_ pass the correct receiving process's `ProcessProposal` check. On the other hand, if there is a deterministic bug in `PrepareProposal` or `ProcessProposal` (or in both), strictly speaking, this makes all processes that hit the bug byzantine. This is a problem in practice, as very often validators are running the Application from the same codebase, so potentially _all_ would likely hit the bug at the same time. This would result in most (or all) processes prevoting `nil`, with the -serious consequences on Tendermint's liveness that this entails. Due to its criticality, Requirement 3 is a +serious consequences on Tendermint's liveness that this entails. Due to its criticality, Requirement 4 is a target for extensive testing and automated verification. -* Requirement 4 [`ProcessProposal`, determinism-1]: `ProcessProposal` is a (deterministic) function of the current +* Requirement 5 [`ProcessProposal`, determinism-1]: `ProcessProposal` is a (deterministic) function of the current state and the block that is about to be applied. In other words, for any correct process $p$, and any arbitrary block $v'$, if $p$'s Tendermint calls `RequestProcessProposal` on $v'$ at height $h$, then $p$'s Application's acceptance or rejection **exclusively** depends on $v'$ and $s_{p,h-1}$. -* Requirement 5 [`ProcessProposal`, determinism-2]: For any two correct processes $p$ and $q$, and any arbitrary block $v'$, +* Requirement 6 [`ProcessProposal`, determinism-2]: For any two correct processes $p$ and $q$, and any arbitrary block $v'$, if $p$'s (resp. $q$'s) Tendermint calls `RequestProcessProposal` on $v'$ at height $h$, then $p$'s Application accepts $v'$ if and only if $q$'s Application accepts $v'$. - Note that this requirement follows from Requirement 4 and the Agreement property of consensus. + Note that this requirement follows from Requirement 5 and the Agreement property of consensus. -Requirements 4 and 5 ensure that all correct processes will react in the same way to a proposed block, even +Requirements 5 and 6 ensure that all correct processes will react in the same way to a proposed block, even if the proposer is Byzantine. However, `ProcessProposal` may contain a bug that renders the acceptance or rejection of the block non-deterministic, and therefore prevents processes hitting -the bug from fulfilling Requirements 4 or 5 (effectively making those processes Byzantine). +the bug from fulfilling Requirements 5 or 6 (effectively making those processes Byzantine). In such a scenario, Tendermint's liveness cannot be guaranteed. Again, this is a problem in practice if most validators are running the same software, as they are likely to hit the bug at the same point. There is currently no clear solution to help with this situation, so @@ -76,43 +85,43 @@ is about to broadcast a non-`nil` precommit message, a correct process can only Let $e^r_p$ be the vote extension that the Application of a correct process $p$ returns via `ResponseExtendVote` in round $r$, height $h$. Let $w^r_p$ be the proposed block that $p$'s Tendermint passes to the Application via `RequestExtendVote` in round $r$, height $h$. -* Requirement 6 [`ExtendVote`, `VerifyVoteExtension`, coherence]: For any two correct processes $p$ and $q$, if $q$ receives $e^r_p$ +* Requirement 7 [`ExtendVote`, `VerifyVoteExtension`, coherence]: For any two correct processes $p$ and $q$, if $q$ receives $e^r_p$ from $p$ in height $h$, $q$'s Application returns Accept in `ResponseVerifyVoteExtension`. -Requirement 6 constrains the creation and handling of vote extensions in a similar way as Requirement 3 +Requirement 7 constrains the creation and handling of vote extensions in a similar way as Requirement 4 contrains the creation and handling of proposed blocks. -Requirement 6 ensures that extensions created by correct processes _always_ pass the `VerifyVoteExtension` +Requirement 7 ensures that extensions created by correct processes _always_ pass the `VerifyVoteExtension` checks performed by correct processes receiving those extensions. However, if there is a (deterministic) bug in `ExtendVote` or `VerifyVoteExtension` (or in both), -we will face the same liveness issues as described for Requirement 3, as Precommit messages with invalid vote +we will face the same liveness issues as described for Requirement 4, as Precommit messages with invalid vote extensions will be discarded. -* Requirement 7 [`VerifyVoteExtension`, determinism-1]: `VerifyVoteExtension` is a (deterministic) function of +* Requirement 8 [`VerifyVoteExtension`, determinism-1]: `VerifyVoteExtension` is a (deterministic) function of the current state, the vote extension received, and the prepared proposal that the extension refers to. In other words, for any correct process $p$, and any arbitrary vote extension $e$, and any arbitrary block $w$, if $p$'s (resp. $q$'s) Tendermint calls `RequestVerifyVoteExtension` on $e$ and $w$ at height $h$, then $p$'s Application's acceptance or rejection **exclusively** depends on $e$, $w$ and $s_{p,h-1}$. -* Requirement 8 [`VerifyVoteExtension`, determinism-2]: For any two correct processes $p$ and $q$, +* Requirement 9 [`VerifyVoteExtension`, determinism-2]: For any two correct processes $p$ and $q$, and any arbitrary vote extension $e$, and any arbitrary block $w$, if $p$'s (resp. $q$'s) Tendermint calls `RequestVerifyVoteExtension` on $e$ and $w$ at height $h$, then $p$'s Application accepts $e$ if and only if $q$'s Application accepts $e$. - Note that this requirement follows from Requirement 7 and the Agreement property of consensus. + Note that this requirement follows from Requirement 8 and the Agreement property of consensus. -Requirements 7 and 8 ensure that the validation of vote extensions will be deterministic at all +Requirements 8 and 9 ensure that the validation of vote extensions will be deterministic at all correct processes. -Requirements 7 and 8 protect against arbitrary vote extension data from Byzantine processes -similarly to Requirements 4 and 5 and proposed blocks. -Requirements 7 and 8 can be violated by a bug inducing non-determinism in +Requirements 8 and 9 protect against arbitrary vote extension data from Byzantine processes +similarly to Requirements 5 and 6 and proposed blocks. +Requirements 8 and 9 can be violated by a bug inducing non-determinism in `VerifyVoteExtension`. In this case liveness can be compromised. Extra care should be put in the implementation of `ExtendVote` and `VerifyVoteExtension` and, as a general rule, `VerifyVoteExtension` _should_ always accept the vote extension. -* Requirement 9 [_all_, no-side-effects]: $p$'s calls to `RequestPrepareProposal`, +* Requirement 10 [_all_, no-side-effects]: $p$'s calls to `RequestPrepareProposal`, `RequestProcessProposal`, `RequestExtendVote`, and `RequestVerifyVoteExtension` at height $h$ do not modify $s_{p,h-1}$. -* Requirement 10 [`ExtendVote`, `FinalizeBlock`, non-dependency]: for any correct process $p$, +* Requirement 11 [`ExtendVote`, `FinalizeBlock`, non-dependency]: for any correct process $p$, and any vote extension $e$ that $p$ received at height $h$, the computation of $s_{p,h}$ does not depend on $e$. @@ -127,13 +136,13 @@ Additionally, >**TODO** I have left out all the "events" as they don't have any impact in safety or liveness >(same for consensus params, and validator set) -* Requirement 11 [`FinalizeBlock`, determinism-1]: For any correct process $p$, +* Requirement 12 [`FinalizeBlock`, determinism-1]: For any correct process $p$, $s_{p,h}$ exclusively depends on $s_{p,h-1}$ and $v_{p,h}$. -* Requirement 12 [`FinalizeBlock`, determinism-2]: For any correct process $p$, +* Requirement 13 [`FinalizeBlock`, determinism-2]: For any correct process $p$, the contents of $T_{p,h}$ exclusively depend on $s_{p,h-1}$ and $v_{p,h}$. -Note that Requirements 11 and 12, combined with Agreement property of consensus ensure +Note that Requirements 12 and 13, combined with Agreement property of consensus ensure the Application state evolves consistently at all correct processes. Finally, notice that neither `PrepareProposal` nor `ExtendVote` have determinism-related diff --git a/spec/abci++/abci++_basic_concepts_002_draft.md b/spec/abci++/abci++_basic_concepts_002_draft.md index cf47dae70..a5d8c9658 100644 --- a/spec/abci++/abci++_basic_concepts_002_draft.md +++ b/spec/abci++/abci++_basic_concepts_002_draft.md @@ -313,8 +313,8 @@ parameters in `FinalizeBlockResponse` are included in the header of the next blo With ABCI++ predecessor, ABCI, the only moment when the Application had access to a block was when it was decided. This led to a block execution model, called _next-block -execution_, where some fields in a block refer to the execution of the previous block, -namely: +execution_, where some fields hashed in a block header refer to the execution of the +previous block, namely: * the merkle root of the Application's state * the transaction results @@ -324,10 +324,10 @@ namely: With ABCI++, an Application may decide to keep using the next-block execution model; however the new methods introduced, `PrepareProposal` and `ProcessProposal` allow for a new execution model, called _same-block execution_. An Application implementing -this execution model, upon receiving a raw proposal via `PrepareProposal` and -potentially modifying the list of transactions, the Application fully executes the -resulting prepared proposal as though it was the decided block. The results of the -block execution are used as follows: +this execution model, upon receiving a raw proposal via `RequestPrepareProposal` +and potentially modifying its transaction list, +fully executes the resulting prepared proposal as though it was the decided block. +The results of the block execution are used as follows: * the Application keeps the events generated and provides them if `FinalizeBlock` is finally called on this prepared proposal. @@ -347,11 +347,34 @@ If the Application decides to keep the next-block execution model, it will not provide any data in `ResponsePrepareProposal`, other than an optionally modified transaction list. -The execution model is set in boolean parameter _same_block_ in ConsensusParameters. +In the long term, the execution model will be set in a new boolean parameter +_same_block_ in `ConsensusParams`. It should **not** be changed once the blockchain has started, unless the Application developers _really_ know what they are doing. - ->**TODO**: Update ConsensusParams struct with "same_block" +However, modifying `ConsensusParams` structure cannot be done lightly if we are to +preserve blockchain compatibility. Therefore we need an interim solution until +soft upgrades are specified and implemented in Tendermint. This somewhat _unsafe_ +solution consists in Tendermint assuming same-block execution if the Application +fills the above mentioned fields in `ResponsePrepareProposal`. + +## Tendermint timeouts in same-block execution + +The new same-block execution mode requires the Application to fully execute the +prepared block at `PrepareProposal` time. This execution is synchronous, so +Tendermint cannot make progress until the Application returns from `PrepareProposal`. +This stands on Tendermint's critical path: if the Application takes a long time +executing the block, the default value of _TimeoutPropose_ might not be sufficient +to accomodate the long block execution time and non-proposer processes might time +out and prevote `nil`, thus starting a further round unnecessarily. + +The application is the best suited to provide a value for _TimeoutPropose_ so +that the block execution time upon `PrepareProposal` fits well in the propose +timeout interval. + +Currently, the Application can override the value of _TimeoutPropose_ via the +`config.toml` file. In the future, `ConsensusParams` may have an extra field +with the current _TimeoutPropose_ value so that the Application has the possibility +to adapt it at every height. ## State Sync diff --git a/spec/abci++/abci++_methods_002_draft.md b/spec/abci++/abci++_methods_002_draft.md index 16e3dc93f..a8091d8c0 100644 --- a/spec/abci++/abci++_methods_002_draft.md +++ b/spec/abci++/abci++_methods_002_draft.md @@ -310,7 +310,7 @@ From the App's perspective, they'll probably skip ProcessProposal | consensus_param_updates | [ConsensusParams](#consensusparams) | Changes to consensus-critical gas, size, and other parameters. | 6 | * **Usage**: - * Contains a preliminary block to be proposed, which the Application can modify. + * Contains a preliminary block to be proposed, called _raw block_, which the Application can modify. * The parameters and types of `RequestPrepareProposal` are the same as `RequestProcessProposal` and `RequestFinalizeBlock`. * The header contains the height, timestamp, and more - it exactly matches the @@ -321,7 +321,7 @@ From the App's perspective, they'll probably skip ProcessProposal `ResponsePrepareProposal.tx`. * In same-block execution mode, the Application must provide values for `ResponsePrepareProposal.app_hash`, `ResponsePrepareProposal.tx_result`, `ResponsePrepareProposal.validator_updates`, and - `ResponsePrepareProposal.consensus_param_updates`, as a result of executing the block. + `ResponsePrepareProposal.consensus_param_updates`, as a result of fully executing the block. * The values for `ResponsePrepareProposal.validator_updates`, or `ResponsePrepareProposal.consensus_param_updates` may be empty. In this case, Tendermint will keep the current values. @@ -333,6 +333,9 @@ From the App's perspective, they'll probably skip ProcessProposal * `ResponseFinalizeBlock.consensus_param_updates` returned for block `H` apply to the consensus params for the same block `H`. For more information on the consensus parameters, see the [application spec entry on consensus parameters](../abci/apps.md#consensus-parameters). + * It is the responsibility of the Application to set the right value for _TimeoutPropose_ so that + the (synchronous) execution of the block does not cause other processes to prevote `nil` because + their propose timeout goes off. * In next-block execution mode, Tendermint will ignore parameters `ResponsePrepareProposal.tx_result`, `ResponsePrepareProposal.validator_updates`, and `ResponsePrepareProposal.consensus_param_updates`. * As a result of executing the prepared proposal, the Application may produce header events or transaction events. diff --git a/spec/abci++/abci++_tmint_expected_behavior_002_draft.md b/spec/abci++/abci++_tmint_expected_behavior_002_draft.md index 4c3903fa0..c408d0ab4 100644 --- a/spec/abci++/abci++_tmint_expected_behavior_002_draft.md +++ b/spec/abci++/abci++_tmint_expected_behavior_002_draft.md @@ -188,21 +188,6 @@ Let us now examine the grammar line by line, providing further details. >decide = %s"" >``` -## `ProcessProposal`'s timeout (a.k.a. Zarko's Github comment in Issue#351) - ->**TODO** (to discuss): `PrepareProposal` is called synchronously. `ProcessProposal` may also want to fully process the block synchronously. ->However, they stand on Tendermint's critical path, so the Tendermint's Propose timeout needs to accomodate that. -> ->Idea: Make propose timestamp (currently hardcoded to 3 secs in the Tendermint Go implementation) part of ConsensusParams, ->so the App can adjust it with its knowledge of the time may take to prepare/process the proposal. -> ->This should probably go elsewhere in the spec. ->Also, see tendermint/tendermint#7274 - -## Failure modes - ->**TODO** Is it worth explaining the failure modes? Since we're going for halt, and can't configure them... - ## Adapting existing Applications that use ABCI In some cases, an existing Application using the legacy ABCI may need to be adapted to work with ABCI++