Apply suggestions from code review

Co-authored-by: str4d <thestr4d@gmail.com>

contrib/metrics/supply_check/__init__.py

@@ -27,7 +27,7 @@ def TheoreticalAndEmpirical(zcashd, deltas, height, flag):
     theoreticalSupply = Network(MAINNET).SupplyAfterHeight(height)
     block = zcashd.getblock(str(height), flag)
     measuredSupply = block['chainSupply']['chainValueZat']
-    empiricalMaximum = measuredSupply + deltas.DeviationAtHeight(height)
+    empiricalMaximum = measuredSupply + deltas.DeviationUpToHeight(height)
     return (theoreticalSupply, empiricalMaximum, block)
 
 # Returns `True` if the theoretical supply matches the empirically
@@ -62,7 +62,7 @@ def main():
     if os.environ.get('ZCASHD_RPC_HOST') is None:
         missing_env.append('  ZCASHD_RPC_HOST: hostname where zcashd is running')
     if os.environ.get('ZCASHD_RPC_PORT') is None:
-        missing_env.append('  ZCASHD_RPC_PORT: zcashd RPC API port (usually 3232 for mainnet)')
+        missing_env.append('  ZCASHD_RPC_PORT: zcashd RPC API port (usually 8232 for mainnet)')
 
     if len(missing_env) > 0:
         print("Please ensure that the following environment variables have been set:")
@@ -78,8 +78,8 @@ def main():
     ))
 
     latestHeight = zcashd.getblockchaininfo()['blocks']
-    deltas = MainnetSupplyDeltas(zcashd)
-    (theoretical, empirical, block) = TheoreticalAndEmpirical(zcashd, deltas, latestHeight, 1)
+    deltas = MainnetSupplyDeltas()
+    (theoretical, empirical, block) = TheoreticalAndEmpirical(zcashd, deltas, latestHeight, TXIDS_ONLY)
     interrupted = False
     if theoretical != empirical:
         with progressbar.ProgressBar(max_value = latestHeight, redirect_stdout = True) as bar:

contrib/metrics/supply_check/supply_deltas.py

@@ -2,8 +2,7 @@ import pprint
 import bisect
 
 class SupplyDeltas:
-    def __init__(self, zcashd, fr_addrs, miner_deltas, flush_interval = 500):
-        self.zcashd = zcashd
+    def __init__(self, fr_addrs, miner_deltas, flush_interval = 500):
         self.fr_addrs = fr_addrs
         self.miner_deltas = miner_deltas
 
@@ -15,13 +14,16 @@ class SupplyDeltas:
         for (deltaHeight, delta) in sorted(deltas_flat):
             self.AddSupplyDelta(deltaHeight, delta)
 
+    # AddSupplyDelta must be called with heights in increasing order.
+    # It will raise an assertion error if an out-of-order insertion is
+    # attempted.
     def AddSupplyDelta(self, deltaHeight, delta):
         assert len(self.delta_cache) == 0 or deltaHeight > self.delta_cache[-1][0]
 
         self.delta_total += delta
-        self.delta_cache.append((deltaHeight, self.delta_total))
+        bisect.insort(self.delta_cache, (deltaHeight, self.delta_total), key=lambda x: x[0])
         
-    def DeviationAtHeight(self, height):
+    def DeviationUpToHeight(self, height):
         i = bisect.bisect(self.delta_cache, height, key=lambda x: x[0])
         return 0 if i == 0 else self.delta_cache[i - 1][1]
     
@@ -46,7 +48,7 @@ class SupplyDeltas:
                 self.AddSupplyDelta(height, delta)
                 if len(self.delta_cache) % 500 == 0:
                     with open("delta_cache.{}.out".format(len(self.delta_cache)), 'w', encoding="utf8") as f:
-                        pprint.pprint(self.miner_deltas, stream = f, indent = 4)
+                        pprint.pprint(self.miner_deltas, stream = f, compact = True)
 
                 return True
 
@@ -55,4 +57,4 @@ class SupplyDeltas:
 
     def PrintDeltas(self):
         with open("delta_cache.out", 'w', encoding="utf8") as f:
-            pprint.pprint(self.miner_deltas, stream = f, indent = 4)
+            pprint.pprint(self.miner_deltas, stream = f, compact = True)

qa/rpc-tests/wallet_persistence.py

@@ -40,7 +40,7 @@ class WalletPersistenceTest (BitcoinTestFramework):
         pre_halving_blocks = 143
         pre_halving_subsidy = Decimal('12.5')
         post_halving_blocks = 57
-        post_halving_subsidy = Decimal('6.25')
+        post_halving_subsidy = pre_halving_subsidy / 2
         expected_supply = (pre_halving_blocks * pre_halving_subsidy +
                            post_halving_blocks * post_halving_subsidy)
 
@@ -86,7 +86,7 @@ class WalletPersistenceTest (BitcoinTestFramework):
         # Verify size of pools after restarting
         chainInfo = self.nodes[0].getblockchaininfo()
         pools = chainInfo['valuePools']
-        # Reenable these test in v5.4.0-rc1
+        # Reenable these test in v5.4.0-rc2
         # check_chain_value(chainInfo['chainSupply'], None, expected_supply)  # Supply
         # check_chain_value(pools[0], 'transparent', expected_supply)
         check_chain_value(pools[1], 'sprout',  Decimal('0'))
@@ -111,7 +111,7 @@ class WalletPersistenceTest (BitcoinTestFramework):
         # Verify size of pools
         chainInfo = self.nodes[0].getblockchaininfo()
         pools = chainInfo['valuePools']
-        # Reenable these tests in v5.4.0-rc1
+        # Reenable these tests in v5.4.0-rc2
         # check_chain_value(chainInfo['chainSupply'], None, expected_supply)  # Supply
         # check_chain_value(pools[0], 'transparent', expected_supply - Decimal('20'))  # Transparent
         check_chain_value(pools[1], 'sprout',  Decimal('0'))  
@@ -126,7 +126,7 @@ class WalletPersistenceTest (BitcoinTestFramework):
         # Verify size of pools
         chainInfo = self.nodes[0].getblockchaininfo()
         pools = chainInfo['valuePools']
-        # Reenable these tests in v5.4.0-rc1
+        # Reenable these tests in v5.4.0-rc2
         # check_chain_value(chainInfo['chainSupply'], None, expected_supply)  # Supply
         # check_chain_value(pools[0], 'transparent', expected_supply - Decimal('20'))  # Transparent
         check_chain_value(pools[1], 'sprout',  Decimal('0')) 

src/chain.h

@@ -23,7 +23,7 @@ static const int SPROUT_VALUE_VERSION = 1001400;
 static const int SAPLING_VALUE_VERSION = 1010100;
 static const int CHAIN_HISTORY_ROOT_VERSION = 2010200;
 static const int NU5_DATA_VERSION = 4050000;
-static const int TRANSPARENT_VALUE_VERSION = 5040000;
+static const int TRANSPARENT_VALUE_VERSION = 5040026;
 
 /**
  * Maximum amount of time that a block timestamp is allowed to be ahead of the
@@ -262,8 +262,9 @@ public:
 
     //! (memory only) Total chain supply up to and including this block.
     //!
-    //! Will be std::nullopt until a reindex has taken place, if nChainTx is
-    //! zero, or if the block has never been connected to a chain tip.
+    //! Will be std::nullopt until a reindex has taken place.
+    //! Will be std::nullopt if nChainTx is zero, or if the block has never been
+    //! connected to a chain tip.
     std::optional<CAmount> nChainTotalSupply;
 
     //! Change in value in the transparent pool produced by the action of the