feat: [ADR-070] Unordered Transactions (1/2)

docs/architecture/adr-070-unordered-account.md

@@ -2,7 +2,7 @@
 
 ## Changelog
 
-* Dec 4, 2023: Initial Draft
+* Dec 4, 2023: Initial Draft (@yihuang, @tac0turtle, @alexanderbez)
 
 ## Status
 
@@ -53,108 +53,147 @@ message TxBody {
 }
 ```
 
-### `DedupTxHashManager`
+### Replay Protection
+
+In order to provide replay protection, a user should ensure that the transaction's
+TTL value is relatively short-lived but long enough to provide enough time to be
+included in a block, e.g. ~H+50. 
+
+We facilitate this by storing the transaction's hash in a durable map, `UnorderedTxManager`,
+to prevent duplicates, i.e. replay attacks. Upon transaction ingress during `CheckTx`,
+we check if the transaction's hash exists in this map or if the TTL value is stale,
+i.e. before the current block. If so, we reject it. Upon inclusion in a block
+during `DeliverTx`, the transaction's hash is set in the map along with it's TTL
+value.
+
+This map is evaluated at the end of each block, e.g. ABCI `Commit`, and all stale
+transactions, i.e. transactions's TTL value who's now beyond the committed block,
+are purged from the map.
+
+An important point to note is that in theory, it may be possible to submit an unordered
+transaction twice, or multiple times, before the transaction is included in a block.
+However, we'll note a few important layers of protection and mitigation:
+
+* Assuming CometBFT is used as the underlying consensus engine and a non-noop mempool
+  is used, CometBFT will reject the duplicate for you.
+* For applications that leverage ABCI++, `ProcessProposal` should evaluate and reject
+  malicious proposals with duplicate transactions.
+* For applications that leverage their own application mempool, their mempool should
+  reject the duplicate for you.
+* Finally, worst case if the duplicate transaction is somehow selected for a block
+  proposal, 2nd and all further attempts to evaluate it, will fail during `DeliverTx`,
+  so worst case you just end up filling up block space with a duplicate transaction.
 
 ```golang
+type TxHash [32]byte
+
 const PurgeLoopSleepMS = 500
 
-// DedupTxHashManager contains the tx hash dictionary for duplicates checking,
-// and expire them when block number progresses.
-type DedupTxHashManager struct {
-  mutex sync.RWMutex
-  // tx hash -> expire block number
-  // for duplicates checking and expiration
-  hashes map[TxHash]uint64
-  // channel to receive latest block numbers
+// UnorderedTxManager contains the tx hash dictionary for duplicates checking,
+// and expire them when block production progresses.
+type UnorderedTxManager struct {
+  // blockCh defines a channel to receive newly committed block heights
   blockCh chan uint64
+
+  mu sync.RWMutex
+	// txHashes defines a map from tx hash -> TTL value, which is used for duplicate
+	// checking and replay protection, as well as purging the map when the TTL is
+	// expired.
+	txHashes map[TxHash]uint64
 }
 
-func NewDedupTxHashManager() *DedupTxHashManager {
-  m := &DedupTxHashManager{
-    hashes: make(map[TxHash]uint64),
-    blockCh: make(ch *uint64, 16),
+func NewUnorderedTxManager() *UnorderedTxManager {
+  m := &UnorderedTxManager{
+		blockCh:  make(chan uint64, 16),
+		txHashes: make(map[TxHash]uint64),
   }
+
+ return m
+}
+
+func (m *UnorderedTxManager) Start() {
   go m.purgeLoop()
-  return m
 }
 
-func (dtm *DedupTxHashManager) Close() error {
-  close(dtm.blockCh)
-  dtm.blockCh = nil
+func (m *UnorderedTxManager) Close() error {
+  close(m.blockCh)
+  m.blockCh = nil
   return nil
 }
 
-func (dtm *DedupTxHashManager) Contains(hash TxHash) (ok bool) {
-  dtm.mutex.RLock()
-  defer dtm.mutex.RUnlock()
+func (m *UnorderedTxManager) Contains(hash TxHash)  bool{
+  m.mu.RLock()
+  defer m.mu.RUnlock()
 
-  _, ok = dtm.hashes[hash]
-  return
+  _, ok := m.txHashes[hash]
+  return ok
 }
 
-func (dtm *DedupTxHashManager) Size() int {
-  dtm.mutex.RLock()
-  defer dtm.mutex.RUnlock()
+func (m *UnorderedTxManager) Size() int {
+  m.mu.RLock()
+  defer m.mu.RUnlock()
 
-  return len(dtm.hashes)
+  return len(m.txHashes)
 }
 
-func (dtm *DedupTxHashManager) Add(hash TxHash, expire uint64) (ok bool) {
-  dtm.mutex.Lock()
-  defer dtm.mutex.Unlock()
+func (m *UnorderedTxManager) Add(hash TxHash, expire uint64) {
+  m.mu.Lock()
+  defer m.mu.Unlock()
+
+  m.txHashes[hash] = expire
+}
 
-  dtm.hashes[hash] = expire
-  return
+// OnNewBlock send the latest block number to the background purge loop, which
+// should be called in ABCI Commit event.
+func (m *UnorderedTxManager) OnNewBlock(blockHeight uint64) {
+  m.blockCh <- blockHeight
+}
+
+// expiredTxs returns expired tx hashes based on the provided block height.
+func (m *UnorderedTxManager) expiredTxs(blockHeight uint64) []TxHash {
+  m.mu.RLock()
+  defer m.mu.RUnlock()
+
+  var result []TxHash
+  for txHash, expire := range m.txHashes {
+    if blockHeight > expire {
+      result = append(result, txHash)
+    }
+  }
+
+  return result
 }
 
-// OnNewBlock send the latest block number to the background purge loop,
-// it should be called in abci commit event.
-func (dtm *DedupTxHashManager) OnNewBlock(blockNumber uint64) {
-  dtm.blockCh <- &blockNumber
+func (m *UnorderedTxManager) purge(txHashes []TxHash) {
+  m.mu.Lock()
+  defer m.mu.Unlock()
+
+  for _, txHash := range txHashes {
+    delete(m.txHashes, txHash)
+  }
 }
 
-// purgeLoop removes expired tx hashes at background
-func (dtm *DedupTxHashManager) purgeLoop() error {
+
+// purgeLoop removes expired tx hashes in the background
+func (m *UnorderedTxManager) purgeLoop() error {
   for {
-    blocks := channelBatchRecv(dtm.blockCh)
+    blocks := channelBatchRecv(m.blockCh)
     if len(blocks) == 0 {
       // channel closed
       break
     }
 
     latest := *blocks[len(blocks)-1]
-    hashes := dtm.expired(latest)
+    hashes := m.expired(latest)
     if len(hashes) > 0 {
-      dtm.purge(hashes)
+      m.purge(hashes)
     }
 
     // avoid burning cpu in catching up phase
     time.Sleep(PurgeLoopSleepMS * time.Millisecond)
   }
 }
 
-// expired find out expired tx hashes based on latest block number
-func (dtm *DedupTxHashManager) expired(block uint64) []TxHash {
-  dtm.mutex.RLock()
-  defer dtm.mutex.RUnlock()
-
-  var result []TxHash
-  for h, expire := range dtm.hashes {
-    if block > expire {
-      result = append(result, h)
-    }
-  }
-  return result
-}
-
-func (dtm *DedupTxHashManager) purge(hashes []TxHash) {
-  dtm.mutex.Lock()
-  defer dtm.mutex.Unlock()
-
-  for _, hash := range hashes {
-    delete(dtm.hashes, hash)
-  }
-}
 
 // channelBatchRecv try to exhaust the channel buffer when it's not empty,
 // and block when it's empty.
@@ -176,35 +215,38 @@ func channelBatchRecv[T any](ch <-chan *T) []*T {
 }
 ```
 
-### Ante Handlers
+### AnteHandler Decorator
 
-Bypass the nonce decorator for un-ordered transactions.
+In order to facilitate bypassing nonce verification, we have to modify the existing
+`IncrementSequenceDecorator` AnteHandler decorator to skip the nonce verification
+when the transaction is marked as un-ordered.
 
 ```golang
 func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
   if tx.UnOrdered() {
     return next(ctx, tx, simulate)
   }
 
-  // the previous logic
+  // ...
 }
 ```
 
-A decorator for the new logic.
+In addition, we need to introduce a new decorator to perform the un-ordered transaction
+verification and map lookup.
 
 ```golang
-type TxHash [32]byte
-
 const (
-  // MaxUnOrderedTTL defines the maximum ttl an un-order tx can set
-  MaxUnOrderedTTL = 1024
+	// DefaultMaxUnOrderedTTL defines the default maximum TTL an un-ordered transaction
+	// can set.
+	DefaultMaxUnOrderedTTL = 1024
 )
 
 type DedupTxDecorator struct {
-  m *DedupTxHashManager
+  m *UnorderedTxManager
+  maxUnOrderedTTL uint64
 }
 
-func (dtd *DedupTxDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
+func (d *DedupTxDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
   // only apply to un-ordered transactions
   if !tx.UnOrdered() {
     return next(ctx, tx, simulate)
@@ -214,18 +256,18 @@ func (dtd *DedupTxDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate boo
     return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "unordered tx must set timeout-height")
   }
 
-  if tx.TimeoutHeight() > ctx.BlockHeight() + MaxUnOrderedTTL {
-    return nil, errorsmod.Wrapf(sdkerrors.ErrLogic, "unordered tx ttl exceeds %d", MaxUnOrderedTTL)
+  if tx.TimeoutHeight() > ctx.BlockHeight() + d.maxUnOrderedTTL {
+    return nil, errorsmod.Wrapf(sdkerrors.ErrLogic, "unordered tx ttl exceeds %d", d.maxUnOrderedTTL)
   }
 
   // check for duplicates
-  if dtd.m.Contains(tx.Hash()) {
+  if d.m.Contains(tx.Hash()) {
     return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "tx is duplicated")
   }
 
   if !ctx.IsCheckTx() {
-    // a new tx included in the block, add the hash to the dictionary
-    dtd.m.Add(tx.Hash(), tx.TimeoutHeight())
+    // a new tx included in the block, add the hash to the unordered tx manager
+    d.m.Add(tx.Hash(), tx.TimeoutHeight())
   }
 
   return next(ctx, tx, simulate)
@@ -234,11 +276,11 @@ func (dtd *DedupTxDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate boo
 
 ### `OnNewBlock`
 
-Wire the `OnNewBlock` method of `DedupTxHashManager` into the BaseApp's ABCI Commit event.
+Wire the `OnNewBlock` method of `UnorderedTxManager` into the BaseApp's ABCI `Commit` event.
 
 ### Start Up
 
-On start up, the node needs to re-fill the tx hash dictionary of `DedupTxHashManager`
+On start up, the node needs to re-fill the tx hash dictionary of `UnorderedTxManager`
 by scanning `MaxUnOrderedTTL` number of historical blocks for existing un-expired
 un-ordered transactions.
 

proto/cosmos/tx/v1beta1/tx.proto

@@ -105,13 +105,29 @@ message TxBody {
 
   // memo is any arbitrary note/comment to be added to the transaction.
   // WARNING: in clients, any publicly exposed text should not be called memo,
-  // but should be called `note` instead (see https://github.com/cosmos/cosmos-sdk/issues/9122).
+  // but should be called `note` instead (see
+  // https://github.com/cosmos/cosmos-sdk/issues/9122).
   string memo = 2;
 
-  // timeout is the block height after which this transaction will not
-  // be processed by the chain
+  // timeout_height is the block height after which this transaction will not
+  // be processed by the chain.
+  //
+  // Note, if unordered=true this value MUST be set
+  // and will act as a short-lived TTL in which the transaction is deemed valid
+  // and kept in memory to prevent duplicates.
   uint64 timeout_height = 3;
 
+  // unordered, when set to true, indicates that the transaction signer(s)
+  // intend for the transaction to be evaluated and executed in an un-ordered
+  // fashion. Specifically, the account's nonce will NOT be checked or
+  // incremented, which allows for fire-and-forget as well as concurrent
+  // transaction execution.
+  //
+  // Note, when set to true, the existing 'timeout_height' value must be set and
+  // will be used to correspond to a height in which the transaction is deemed
+  // valid.
+  bool unordered = 4;
+
   // extension_options are arbitrary options that can be added by chains
   // when the default options are not sufficient. If any of these are present
   // and can't be handled, the transaction will be rejected
@@ -211,14 +227,16 @@ message Fee {
   // before an out of gas error occurs
   uint64 gas_limit = 2;
 
-  // if unset, the first signer is responsible for paying the fees. If set, the specified account must pay the fees.
-  // the payer must be a tx signer (and thus have signed this field in AuthInfo).
-  // setting this field does *not* change the ordering of required signers for the transaction.
+  // if unset, the first signer is responsible for paying the fees. If set, the
+  // specified account must pay the fees. the payer must be a tx signer (and
+  // thus have signed this field in AuthInfo). setting this field does *not*
+  // change the ordering of required signers for the transaction.
   string payer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 
-  // if set, the fee payer (either the first signer or the value of the payer field) requests that a fee grant be used
-  // to pay fees instead of the fee payer's own balance. If an appropriate fee grant does not exist or the chain does
-  // not support fee grants, this will fail
+  // if set, the fee payer (either the first signer or the value of the payer
+  // field) requests that a fee grant be used to pay fees instead of the fee
+  // payer's own balance. If an appropriate fee grant does not exist or the
+  // chain does not support fee grants, this will fail
   string granter = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }