github.com/ari-anchor/sei-tendermint@v0.0.0-20230519144642-dc826b7b56bb/docs/architecture/adr-029-check-tx-consensus.md (about)

     1  # ADR 029: Check block txs before prevote
     2  
     3  ## Changelog
     4  
     5  04-10-2018: Update with link to issue
     6  [#2384](https://github.com/tendermint/tendermint/issues/2384) and reason for rejection
     7  19-09-2018: Initial Draft
     8  
     9  ## Context
    10  
    11  We currently check a tx's validity through 2 ways.
    12  
    13  1. Through checkTx in mempool connection.
    14  2. Through deliverTx in consensus connection.
    15  
    16  The 1st is called when external tx comes in, so the node should be a proposer this time. The 2nd is called when external block comes in and reach the commit phase, the node doesn't need to be the proposer of the block, however it should check the txs in that block.
    17  
    18  In the 2nd situation, if there are many invalid txs in the block, it would be too late for all nodes to discover that most txs in the block are invalid, and we'd better not record invalid txs in the blockchain too.
    19  
    20  ## Proposed solution
    21  
    22  Therefore, we should find a way to check the txs' validity before send out a prevote. Currently we have cs.isProposalComplete() to judge whether a block is complete. We can have
    23  
    24  ```
    25  func (blockExec *BlockExecutor) CheckBlock(block *types.Block) error {
    26     // check txs of block.
    27     for _, tx := range block.Txs {
    28        reqRes := blockExec.proxyApp.CheckTxAsync(tx)
    29        reqRes.Wait()
    30        if reqRes.Response == nil || reqRes.Response.GetCheckTx() == nil || reqRes.Response.GetCheckTx().Code != abci.CodeTypeOK {
    31           return errors.Errorf("tx %v check failed. response: %v", tx, reqRes.Response)
    32        }
    33     }
    34     return nil
    35  }
    36  ```
    37  
    38  such a method in BlockExecutor to check all txs' validity in that block.
    39  
    40  However, this method should not be implemented like that, because checkTx will share the same state used in mempool in the app.  So we should define a new interface method checkBlock in Application to indicate it to use the same state as deliverTx.
    41  
    42  ```
    43  type Application interface {
    44     // Info/Query Connection
    45     Info(RequestInfo) ResponseInfo                // Return application info
    46     Query(RequestQuery) ResponseQuery             // Query for state
    47  
    48     // Mempool Connection
    49     CheckTx(tx []byte) ResponseCheckTx // Validate a tx for the mempool
    50  
    51     // Consensus Connection
    52     InitChain(RequestInitChain) ResponseInitChain // Initialize blockchain with validators and other info from TendermintCore
    53     CheckBlock(RequestCheckBlock) ResponseCheckBlock
    54     BeginBlock(RequestBeginBlock) ResponseBeginBlock // Signals the beginning of a block
    55     DeliverTx(tx []byte) ResponseDeliverTx           // Deliver a tx for full processing
    56     EndBlock(RequestEndBlock) ResponseEndBlock       // Signals the end of a block, returns changes to the validator set
    57     Commit() ResponseCommit                          // Commit the state and return the application Merkle root hash
    58  }
    59  ```
    60  
    61  All app should implement that method. For example, counter:
    62  
    63  ```
    64  func (app *CounterApplication) CheckBlock(block types.Request_CheckBlock) types.ResponseCheckBlock {
    65     if app.serial {
    66     	  app.originalTxCount = app.txCount   //backup the txCount state
    67        for _, tx := range block.CheckBlock.Block.Txs {
    68           if len(tx) > 8 {
    69              return types.ResponseCheckBlock{
    70                 Code: code.CodeTypeEncodingError,
    71                 Log:  fmt.Sprintf("Max tx size is 8 bytes, got %d", len(tx))}
    72           }
    73           tx8 := make([]byte, 8)
    74           copy(tx8[len(tx8)-len(tx):], tx)
    75           txValue := binary.BigEndian.Uint64(tx8)
    76           if txValue < uint64(app.txCount) {
    77              return types.ResponseCheckBlock{
    78                 Code: code.CodeTypeBadNonce,
    79                 Log:  fmt.Sprintf("Invalid nonce. Expected >= %v, got %v", app.txCount, txValue)}
    80           }
    81           app.txCount++
    82        }
    83     }
    84     return types.ResponseCheckBlock{Code: code.CodeTypeOK}
    85  }
    86  ```
    87  
    88  In BeginBlock, the app should restore the state to the orignal state before checking the block:
    89  
    90  ```
    91  func (app *CounterApplication) DeliverTx(tx []byte) types.ResponseDeliverTx {
    92     if app.serial {
    93        app.txCount = app.originalTxCount   //restore the txCount state
    94     }
    95     app.txCount++
    96     return types.ResponseDeliverTx{Code: code.CodeTypeOK}
    97  }
    98  ```
    99  
   100  The txCount is like the nonce in ethermint, it should be restored when entering the deliverTx phase. While some operation like checking the tx signature needs not to be done again. So the deliverTx can focus on how a tx can be applied, ignoring the checking of the tx, because all the checking has already been done in the checkBlock phase before.
   101  
   102  An optional optimization is alter the deliverTx to deliverBlock. For the block has already been checked by checkBlock, so all the txs in it are valid. So the app can cache the block, and in the deliverBlock phase, it just needs to apply the block in the cache. This optimization can save network current in deliverTx.
   103  
   104  
   105  
   106  ## Status
   107  
   108  Rejected
   109  
   110  ## Decision
   111  
   112  Performance impact is considered too great. See [#2384](https://github.com/tendermint/tendermint/issues/2384)
   113  
   114  ## Consequences
   115  
   116  ### Positive
   117  
   118  - more robust to defend the adversary to propose a block full of invalid txs.
   119  
   120  ### Negative
   121  
   122  - add a new interface method. app logic needs to adjust to appeal to it.
   123  - sending all the tx data over the ABCI twice
   124  - potentially redundant validations (eg. signature checks in both CheckBlock and
   125    DeliverTx)
   126  
   127  ### Neutral