This repository was archived by the owner on Feb 23, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 54
This repository was archived by the owner on Feb 23, 2022. It is now read-only.
ABCI++ comments that needs to be addressed from https://github.com/tendermint/spec/pull/327 #351
Copy link
Copy link
Closed
Description
- Logic for
OnTimeoutProposeneeds to be added. - Explanations for
ReceiveProposalrules are not correct (https://github.com/tendermint/spec/pull/327/files#r710104159, https://github.com/tendermint/spec/pull/327/files#r710107194). - We should clarify the semantic of
valid(v)function, i.e., is it done purely at the Tendermint level (basically block header verification), or we assume it requires also application level checks. - Why we use
getBlockProposal(txdata)in v0 andgetUnpreparedBlockProposal(txdata)in v2: https://github.com/tendermint/spec/pull/327/files#r711591189 - Might be good idea adding note that
PrepareProposalis adding latency on the critical path, and therefore,timeoutProposewould need to be updated accordingly. One idea would be allowing application to provide some hint to the Tendermint regarding duration ofPrepareProposalso thetimeoutProposecould be estimated more correctly (https://github.com/tendermint/spec/pull/327/files#r711591018). - I would suggest storing the result of
ABCI.ProcessProposalin a local variable and using it at line 52, as this presentation might suggest two ABCI calls (https://github.com/tendermint/spec/pull/327/files#r711591437) - Clarify benefits of evidence handling as part of ABCI processing logic (https://github.com/tendermint/spec/pull/327/files#r711591744)
- We need also precommit_extension field as part of PRECOMMIT message (https://github.com/tendermint/spec/pull/327/files#r711592538)
- From semantic perspective this presentation is not ideal as upon rules are atomic, so blocking within upon rule prevents any other rule being triggered. If we want to separate sending/receiving proposal_header from the proposal_data, we should probably introduce two message types (https://github.com/tendermint/spec/pull/327/files#r711593978)
- PROPOSAL message contains full block proposal in this rule, while above it is only header. As mentioned above, we can make it clear by having in addition to PROPOSAL, also PROPOSAL_HEADER message and then have a separate rule for PROPOSAL_HEADER message (https://github.com/tendermint/spec/pull/327/files#r711594462).
- I would prefer using slightly different semantics to signal it is asynchronous computation: we can distinct between synchronous ABCI calls where we are synchronously waiting for result, or asynchronous where we are essentially sending a request and will receive reply later (eventually). I don't have strong preference how we can capture this, maybe using Future or Promise as a return value. Then instead of Join we can have upon rule contains condition based on response. The benefit is that there is no blocking wait inside the upon rule as it semantically prevents other upon rules being triggered (https://github.com/tendermint/spec/pull/327/files#r711618984)
- Clarify the semantics of Join (https://github.com/tendermint/spec/pull/327/files#r711619497).
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels