From f4e10398309f561b087b11ca87a2b3c2c9c7aca7 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Wed, 12 Jan 2022 14:35:45 +0100 Subject: [PATCH] Addressed outstanding comments --- .../abci++_app_requirements_002_draft.md | 6 ++++- .../abci++/abci++_basic_concepts_002_draft.md | 10 ++++---- spec/abci++/abci++_methods_002_draft.md | 23 +++++++++---------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/spec/abci++/abci++_app_requirements_002_draft.md b/spec/abci++/abci++_app_requirements_002_draft.md index f931537b4..994e1999d 100644 --- a/spec/abci++/abci++_app_requirements_002_draft.md +++ b/spec/abci++/abci++_app_requirements_002_draft.md @@ -31,7 +31,11 @@ compute various hashes in the block header that will finally be part of the prop mode, $p$'s Application does not provide values for the following parameters in `ResponsePrepareProposal`: _AppHash_, _TxResults_, _ConsensusParams_, _ValidatorUpdates_. -In practical terms, Requirement 2 implies that Tendermint will ignore those parameters in `ResponsePrepareProposal`. +In practical terms, Requirements 1 and 2 imply that Tendermint will (a) panic if the Application is in +same-block execution mode and _does not_ provide values for +_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$, if $q$'s Tendermint calls `RequestProcessProposal` on $v'_p$, diff --git a/spec/abci++/abci++_basic_concepts_002_draft.md b/spec/abci++/abci++_basic_concepts_002_draft.md index 6862b61bc..cf47dae70 100644 --- a/spec/abci++/abci++_basic_concepts_002_draft.md +++ b/spec/abci++/abci++_basic_concepts_002_draft.md @@ -48,7 +48,7 @@ More details on managing state across connections can be found in the section on ## Errors The `Query`, and `CheckTx` methods include a `Code` field in their `Response*`. -The `Code` field is also included in type `DeliverTxResult`, used by +The `Code` field is also included in type `TxResult`, used by method `FinalizeBlock`'s `Response*`. Field `Code` is meant to contain an application-specific response code. A response code of `0` indicates no error. Any other response code @@ -65,7 +65,7 @@ of these methods, the Application must crash to ensure that the error is safely handled by an operator. Method `FinalizeBlock` is a special case. It contains a number of -`Code` and `Codespace` fields as part of type `DeliverTxResult`. Each of +`Code` and `Codespace` fields as part of type `TxResult`. Each of these codes reports errors related to the transaction it is attached to. However, `FinalizeBlock` does not return errors at the top level, so the same considerations on critical issues made for `Echo`, `Info`, and @@ -81,10 +81,10 @@ When Tendermint receives a `ResponseCheckTx` with a non-zero `Code`, the associa transaction will not be added to Tendermint's mempool or it will be removed if it is already included. -### `DeliverTxResult` (as part of `FinalizeBlock`) +### `TxResult` (as part of `FinalizeBlock`) -The `DeliverTxResult` type delivers transactions from Tendermint to the Application. -When Tendermint receives a `ResponseFinalizeBlock` containing a `DeliverTxResult` +The `TxResult` type delivers transactions from Tendermint to the Application. +When Tendermint receives a `ResponseFinalizeBlock` containing a `TxResult` with a non-zero `Code`, the response code is logged. The transaction was already included in a block, so the `Code` does not influence Tendermint consensus. diff --git a/spec/abci++/abci++_methods_002_draft.md b/spec/abci++/abci++_methods_002_draft.md index 327769a62..5a6e8963a 100644 --- a/spec/abci++/abci++_methods_002_draft.md +++ b/spec/abci++/abci++_methods_002_draft.md @@ -305,7 +305,7 @@ From the App's perspective, they'll probably skip ProcessProposal | modified_tx | bool | The Application sets it to true to denote it made changes to transactions | 1 | | tx | repeated [TransactionRecord](#transactionrecord) | Possibly modified list of transactions that have been picked as part of the proposed block. | 2 | | app_hash | bytes | The Merkle root hash of the application state. | 3 | - | tx_result | repeated [DeliverTxResult](#delivertxresult) | List of structures containing the data resulting from executing the transactions | 4 | + | tx_result | repeated [TxResult](#txresult) | List of structures containing the data resulting from executing the transactions | 4 | | validator_updates | repeated [ValidatorUpdate](#validatorupdate) | Changes to validator set (set voting power to 0 to remove). | 5 | | consensus_param_updates | [ConsensusParams](#consensusparams) | Changes to consensus-critical gas, size, and other parameters. | 6 | @@ -572,7 +572,7 @@ from this condition, but not sure), and _p_ receives a Precommit message for rou | Name | Type | Description | Field Number | |-------------------------|-------------------------------------------------------------|----------------------------------------------------------------------------------|--------------| | block_events | repeated [Event](abci++_basic_concepts_002_draft.md#events) | Type & Key-Value events for indexing | 1 | - | tx_result | repeated [DeliverTxResult](#delivertxresult) | List of structures containing the data resulting from executing the transactions | 2 | + | tx_result | repeated [TxResult](#txresult) | List of structures containing the data resulting from executing the transactions | 2 | | validator_updates | repeated [ValidatorUpdate](#validatorupdate) | Changes to validator set (set voting power to 0 to remove). | 3 | | consensus_param_updates | [ConsensusParams](#consensusparams) | Changes to consensus-critical gas, size, and other parameters. | 4 | | app_hash | bytes | The Merkle root hash of the application state. | 6 | @@ -591,7 +591,7 @@ from this condition, but not sure), and _p_ receives a Precommit message for rou * `ResponseFinalizeBlock.tx_result[i].Code == 0` only if the _i_-th transaction is fully valid. * In next-block execution mode, the Application must provide values for `ResponseFinalizeBlock.app_hash`, `ResponseFinalizeBlock.tx_result`, `ResponseFinalizeBlock.validator_updates`, and - `ResponsePrepareProposal.consensus_param_updates` as a result of executing the block. + `ResponseFinalizeBlock.consensus_param_updates` as a result of executing the block. * The values for `ResponseFinalizeBlock.validator_updates`, or `ResponseFinalizeBlock.consensus_param_updates` may be empty. In this case, Tendermint will keep the current values. @@ -603,9 +603,9 @@ from this condition, but not sure), and _p_ receives a Precommit message for rou * `ResponseFinalizeBlock.consensus_param_updates` returned for block `H` apply to the consensus params for block `H+1`. For more information on the consensus parameters, see the [application spec entry on consensus parameters](../abci/apps.md#consensus-parameters). - * In same-block execution mode, Tendermint will ignore values for `ResponseFinalizeBlock.app_hash`, - `ResponseFinalizeBlock.tx_result`, `ResponseFinalizeBlock.validator_updates`, and - `ResponsePrepareProposal.consensus_param_updates`, as those data have been provided by `PrepareProposal`. + * In same-block execution mode, Tendermint will log an error and ignore values for + `ResponseFinalizeBlock.app_hash`, `ResponseFinalizeBlock.tx_result`, `ResponseFinalizeBlock.validator_updates`, + and `ResponsePrepareProposal.consensus_param_updates`, as those data have been provided by `PrepareProposal`. * Application is expected to persist its state at the end of this call, before calling `ResponseFinalizeBlock`. * `ResponseFinalizeBlock.app_hash` contains an (optional) Merkle root hash of the application state. * `ResponseFinalizeBlock.app_hash` is included @@ -618,11 +618,10 @@ from this condition, but not sure), and _p_ receives a Precommit message for rou of `RequestFinalizeBlock` and the previous committed state. * Later calls to `Query` can return proofs about the application state anchored in this Merkle root hash. - * Use `retain_height` with caution! If all nodes in the network remove historical - blocks then this data is permanently lost, and no new nodes will be able to - join the network and bootstrap. Historical blocks may also be required for - other purposes, e.g. auditing, replay of non-persisted heights, light client - verification, and so on. + * Use `ResponseFinalizeBlock.retain_height` with caution! If all nodes in the network remove historical + blocks then this data is permanently lost, and no new nodes will be able to join the network and + bootstrap. Historical blocks may also be required for other purposes, e.g. auditing, replay of + non-persisted heights, light client verification, and so on. * Just as `ProcessProposal`, the implementation of `FinalizeBlock` MUST be deterministic, since it is making the Application's state evolve in the context of state machine replication. * Currently, Tendermint will fill up all fields in `RequestFinalizeBlock`, even if they were @@ -780,7 +779,7 @@ Most of the data structures used in ABCI are shared [common data structures](../ ## Data types introduced in ABCI++ -### DeliverTxResult +### TxResult * **Fields**: