review

chain/chain/src/chain.rs

@@ -850,12 +850,23 @@ impl Chain {
             prev_block_hash,
             shard_id,
         )?;
-        // TODO(post-state-root): this misses outgoing receipts from the last block before
-        // the switch to post-state-root. Incoming receipts for that block corresponds to the
-        // outgoing receipts from the previous block, but incoming receipts for the next block
-        // include outgoing receipts for that block. These receipts can be obtained from the db
-        // using get_outgoing_receipts_for_shard since we currently track all shard. This will
-        // be implemented later along with an intergation test to reproduce the issue.
+        // TODO(post-state-root):
+        // This misses outgoing receipts from the last non-post-state-root block B.
+        // Before post-state-root incoming receipts store receipts that are supposed to be applied
+        // in this block, which corresponds to the outgoing receipts from the previous block.
+        // After post-state-root incoming receipts store receipts that are the result of executing
+        // that block, which corresponds to the outgoing receipts from the current block.
+        // So considering which outgoing receipts correspond to the incoming receipts for the blocks:
+        // * ...
+        // * pre-state-root  block B-1: outgoing B-2 -> incoming B-1
+        // * pre-state-root  block B:   outgoing B-1 -> incoming B
+        // * post-state-root block B+1: outgoing B+1 -> incoming B+1
+        // * post-state-root block B+2: outgoing B+2 -> incoming B+2
+        // * ...
+        // We can see that outgoing receipts of block B are not stored anywhere in the incoming receipts.
+        // These receipts can be obtained from the db using get_outgoing_receipts_for_shard since we
+        // currently track all shard. This will be implemented later along with an intergation test
+        // to reproduce the issue.
         let receipts =
             collect_receipts_from_response(&self.store.get_incoming_receipts_for_shard(
                 self.epoch_manager.as_ref(),
@@ -1763,7 +1774,6 @@ impl Chain {
         if !self.care_about_any_shard_or_part(me, *block.header().prev_hash())? {
             return Ok(HashMap::new());
         }
-
         let height = block.header().height();
         let mut receipt_proofs_by_shard_id = HashMap::new();
 

chain/client/src/client.rs

@@ -866,18 +866,6 @@ impl Client {
             last_header.height_included(),
         )?;
 
-        // Receipts proofs root is calculating here
-        //
-        // For each subset of incoming_receipts_into_shard_i_from_the_current_one
-        // we calculate hash here and save it
-        // and then hash all of them into a single receipts root
-        //
-        // We check validity in two ways:
-        // 1. someone who cares about shard will download all the receipts
-        // and checks that receipts_root equals to all receipts hashed
-        // 2. anyone who just asks for one's incoming receipts
-        // will receive a piece of incoming receipts only
-        // with merkle receipts proofs which can be checked locally
         let outgoing_receipts_root = self.calculate_receipts_root(epoch_id, &outgoing_receipts)?;
         let protocol_version = self.epoch_manager.get_epoch_protocol_version(epoch_id)?;
         let gas_used = chunk_extra.gas_used();
@@ -999,27 +987,13 @@ impl Client {
             last_header.height_included(),
         )?;
 
-        // Receipts proofs root is calculating here
-        //
-        // For each subset of incoming_receipts_into_shard_i_from_the_current_one
-        // we calculate hash here and save it
-        // and then hash all of them into a single receipts root
-        //
-        // We check validity in two ways:
-        // 1. someone who cares about shard will download all the receipts
-        // and checks that receipts_root equals to all receipts hashed
-        // 2. anyone who just asks for one's incoming receipts
-        // will receive a piece of incoming receipts only
-        // with merkle receipts proofs which can be checked locally
         let (transaction_receipts_parts, encoded_length) =
             EncodedShardChunk::encode_transaction_receipts(
                 &mut self.rs_for_chunk_production,
                 transactions,
                 &apply_result.outgoing_receipts,
             )
-            .map_err(|err| {
-                Error::ChunkProducer(format!("Failed to encode transactions/receipts: {}", err))
-            })?;
+            .map_err(|err| Error::Chunk(err.into()))?;
         let mut content = EncodedShardChunkBody { parts: transaction_receipts_parts };
         content.reconstruct(&mut self.rs_for_chunk_production).unwrap();
         let (encoded_merkle_root, merkle_paths) = content.get_merkle_hash_and_paths();
@@ -1072,6 +1046,17 @@ impl Client {
         Ok((encoded_chunk, merkle_paths, apply_result.outgoing_receipts))
     }
 
+    /// Calculates the root of receipt proofs.
+    /// All receipts are groupped by receiver_id and hash is calculated
+    /// for each such group. Then we merkalize these hashes to calculate
+    /// the receipts root.
+    ///
+    /// Receipts root is used in the following ways:
+    /// 1. Someone who cares about shard will download all the receipts
+    ///    and checks if those correspond to receipts_root.
+    /// 2. Anyone who asks for one's incoming receipts will receive a piece
+    ///    of incoming receipts only with merkle receipts proofs which can
+    ///    be checked locally.
     fn calculate_receipts_root(
         &self,
         epoch_id: &EpochId,