Apply suggestions from code review

Co-authored-by: Daira Hopwood <daira@jacaranda.org>
This commit is contained in:
Kris Nuttycombe 2023-01-17 19:17:56 -07:00
parent aa78fc0878
commit 2fd52ada51
5 changed files with 40 additions and 36 deletions

View File

@ -60,19 +60,20 @@ class WalletPersistenceTest (BitcoinTestFramework):
addresses = self.nodes[0].z_listaddresses() addresses = self.nodes[0].z_listaddresses()
assert_true(sapling_addr in addresses, "Should contain address before restart") assert_true(sapling_addr in addresses, "Should contain address before restart")
def check_chain_value(pool, expected_value): def check_chain_value(pool, expected_id, expected_value):
assert_equal(pool.get('id', None), expected_id)
assert_equal(pool['monitored'], True)
assert_equal(pool['chainValue'], expected_value) assert_equal(pool['chainValue'], expected_value)
assert_equal(pool['chainValueZat'], expected_value * COIN) assert_equal(pool['chainValueZat'], expected_value * COIN)
# Verify size of pools # Verify size of pools
chainInfo = self.nodes[0].getblockchaininfo() chainInfo = self.nodes[0].getblockchaininfo()
print(str(chainInfo))
assert_equal(chainInfo['chainSupply']['chainValue'], expected_supply) # Supply
pools = chainInfo['valuePools'] pools = chainInfo['valuePools']
check_chain_value(pools[0], expected_supply) # Transparent check_chain_value(chainInfo['chainSupply'], None, expected_supply)
check_chain_value(pools[1], Decimal('0')) # Sprout check_chain_value(pools[0], 'transparent', expected_supply)
check_chain_value(pools[2], Decimal('0')) # Sapling check_chain_value(pools[1], 'sprout', Decimal('0'))
check_chain_value(pools[3], Decimal('0')) # Orchard check_chain_value(pools[2], 'sapling', Decimal('0'))
check_chain_value(pools[3], 'orchard', Decimal('0'))
# Restart the nodes # Restart the nodes
stop_nodes(self.nodes) stop_nodes(self.nodes)
@ -87,11 +88,11 @@ class WalletPersistenceTest (BitcoinTestFramework):
chainInfo = self.nodes[0].getblockchaininfo() chainInfo = self.nodes[0].getblockchaininfo()
pools = chainInfo['valuePools'] pools = chainInfo['valuePools']
# Reenable these test in v5.4.0-rc1 # Reenable these test in v5.4.0-rc1
# check_chain_value(chainInfo['chainSupply'], expected_supply) # Supply # check_chain_value(chainInfo['chainSupply'], None, expected_supply) # Supply
# check_chain_value(pools[0], expected_supply) # Transparent # check_chain_value(pools[0], 'transparent', expected_supply)
check_chain_value(pools[1], Decimal('0')) # Sprout check_chain_value(pools[1], 'sprout', Decimal('0'))
check_chain_value(pools[2], Decimal('0')) # Sapling check_chain_value(pools[2], 'sapling', Decimal('0'))
check_chain_value(pools[3], Decimal('0')) # Orchard check_chain_value(pools[3], 'orchard', Decimal('0'))
# Node 0 shields funds to Sapling address # Node 0 shields funds to Sapling address
taddr0 = get_coinbase_address(self.nodes[0]) taddr0 = get_coinbase_address(self.nodes[0])
@ -112,11 +113,11 @@ class WalletPersistenceTest (BitcoinTestFramework):
chainInfo = self.nodes[0].getblockchaininfo() chainInfo = self.nodes[0].getblockchaininfo()
pools = chainInfo['valuePools'] pools = chainInfo['valuePools']
# Reenable these tests in v5.4.0-rc1 # Reenable these tests in v5.4.0-rc1
# check_chain_value(chainInfo['chainSupply'], expected_supply) # Supply # check_chain_value(chainInfo['chainSupply'], None, expected_supply) # Supply
# check_chain_value(pools[0], expected_supply - Decimal('20')) # Transparent # check_chain_value(pools[0], 'transparent', expected_supply - Decimal('20')) # Transparent
check_chain_value(pools[1], Decimal('0')) # Sprout check_chain_value(pools[1], 'sprout', Decimal('0'))
check_chain_value(pools[2], Decimal('20')) # Sapling check_chain_value(pools[2], 'sapling', Decimal('20'))
check_chain_value(pools[3], Decimal('0')) # Orchard check_chain_value(pools[3], 'orchard', Decimal('0'))
# Restart the nodes # Restart the nodes
stop_nodes(self.nodes) stop_nodes(self.nodes)
@ -127,11 +128,11 @@ class WalletPersistenceTest (BitcoinTestFramework):
chainInfo = self.nodes[0].getblockchaininfo() chainInfo = self.nodes[0].getblockchaininfo()
pools = chainInfo['valuePools'] pools = chainInfo['valuePools']
# Reenable these tests in v5.4.0-rc1 # Reenable these tests in v5.4.0-rc1
# check_chain_value(chainInfo['chainSupply'], expected_supply) # Supply # check_chain_value(chainInfo['chainSupply'], None, expected_supply) # Supply
# check_chain_value(pools[0], expected_supply - Decimal('20')) # Transparent # check_chain_value(pools[0], 'transparent', expected_supply - Decimal('20')) # Transparent
check_chain_value(pools[1], Decimal('0')) # Sprout check_chain_value(pools[1], 'sprout', Decimal('0'))
check_chain_value(pools[2], Decimal('20')) # Sapling check_chain_value(pools[2], 'sapling', Decimal('20'))
check_chain_value(pools[3], Decimal('0')) # Orchard check_chain_value(pools[3], 'orchard', Decimal('0'))
# Node 0 sends some shielded funds to Node 1 # Node 0 sends some shielded funds to Node 1
dest_addr = self.nodes[1].z_getnewaddress('sapling') dest_addr = self.nodes[1].z_getnewaddress('sapling')

View File

@ -994,7 +994,6 @@ CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const
return GetTransparentValueIn(tx) + tx.GetShieldedValueIn(); return GetTransparentValueIn(tx) + tx.GetShieldedValueIn();
} }
// TODO: remove this if it ends up unused
CAmount CCoinsViewCache::GetTransparentValueIn(const CTransaction& tx) const CAmount CCoinsViewCache::GetTransparentValueIn(const CTransaction& tx) const
{ {
if (tx.IsCoinBase()) if (tx.IsCoinBase())

View File

@ -4687,7 +4687,7 @@ bool ReceivedBlockTransactions(
// place that we have a valid coins view with which to compute // place that we have a valid coins view with which to compute
// the transparent input value and fees. // the transparent input value and fees.
// calculate the block's effect on the chain's net Sprout value // Calculate the block's effect on the Sprout chain value pool balance.
if (pindex->pprev->nChainSproutValue && pindex->nSproutValue) { if (pindex->pprev->nChainSproutValue && pindex->nSproutValue) {
pindex->nChainSproutValue = *pindex->pprev->nChainSproutValue + *pindex->nSproutValue; pindex->nChainSproutValue = *pindex->pprev->nChainSproutValue + *pindex->nSproutValue;
} else { } else {

View File

@ -352,6 +352,10 @@ CAmount CTransaction::GetShieldedValueIn() const
} }
} }
if (IsCoinBase() && nValue != 0) {
throw std::runtime_error("CTransaction::GetShieldedValueIn(): shielded value of inputs must be zero in coinbase transactions.");
}
return nValue; return nValue;
} }

View File

@ -747,19 +747,19 @@ UniValue getblock(const UniValue& params, bool fHelp)
" \"difficulty\" : x.xxx, (numeric) The difficulty\n" " \"difficulty\" : x.xxx, (numeric) The difficulty\n"
" \"chainSupply\": { (object) information about the total supply\n" " \"chainSupply\": { (object) information about the total supply\n"
" \"monitored\": xx, (boolean) true if the total supply is being monitored\n" " \"monitored\": xx, (boolean) true if the total supply is being monitored\n"
" \"chainValue\": xxxxxx, (numeric, optional) total chain supply\n" " \"chainValue\": xxxxxx, (numeric, optional) total chain supply after this block, in " + CURRENCY_UNIT + "\n"
" \"chainValueZat\": xxxxxx, (numeric, optional) total chain supply in satoshis\n" " \"chainValueZat\": xxxxxx, (numeric, optional) total chain supply after this block, in " + MINOR_CURRENCY_UNIT + "\n"
" \"valueDelta\": xxxxxx, (numeric, optional) change to the chain supply produced by this block\n" " \"valueDelta\": xxxxxx, (numeric, optional) change to the chain supply produced by this block, in " + CURRENCY_UNIT + "\n"
" \"valueDeltaZat\": xxxxxx, (numeric, optional) change to the chain supply produced by this block, in satoshis\n" " \"valueDeltaZat\": xxxxxx, (numeric, optional) change to the chain supply produced by this block, in " + MINOR_CURRENCY_UNIT + "\n"
" }\n" " }\n"
" \"valuePools\": [ (array) information about each value pool\n" " \"valuePools\": [ (array) information about each value pool\n"
" {\n" " {\n"
" \"id\": \"xxxx\", (string) name of the pool\n" " \"id\": \"xxxx\", (string) name of the pool\n"
" \"monitored\": xx, (boolean) true if the pool is being monitored\n" " \"monitored\": xx, (boolean) true if the pool is being monitored\n"
" \"chainValue\": xxxxxx, (numeric, optional) total amount in the pool\n" " \"chainValue\": xxxxxx, (numeric, optional) total amount in the pool, in " + CURRENCY_UNIT + "\n"
" \"chainValueZat\": xxxxxx, (numeric, optional) total amount in the pool in satoshis\n" " \"chainValueZat\": xxxxxx, (numeric, optional) total amount in the pool, in " + MINOR_CURRENCY_UNIT + "\n"
" \"valueDelta\": xxxxxx, (numeric, optional) change to the amount in the pool produced by this block\n" " \"valueDelta\": xxxxxx, (numeric, optional) change to the amount in the pool produced by this block, in " + CURRENCY_UNIT + "\n"
" \"valueDeltaZat\": xxxxxx, (numeric, optional) change to the amount in the pool produced by this block, in satoshis\n" " \"valueDeltaZat\": xxxxxx, (numeric, optional) change to the amount in the pool produced by this block, in " + MINOR_CURRENCY_UNIT + "\n"
" }, ...\n" " }, ...\n"
" ]\n" " ]\n"
" \"previousblockhash\" : \"hash\", (string) The hash of the previous block\n" " \"previousblockhash\" : \"hash\", (string) The hash of the previous block\n"
@ -1058,15 +1058,15 @@ UniValue getblockchaininfo(const UniValue& params, bool fHelp)
" \"commitments\": xxxxxx, (numeric) the current number of note commitments in the commitment tree\n" " \"commitments\": xxxxxx, (numeric) the current number of note commitments in the commitment tree\n"
" \"chainSupply\": { (object) information about the total supply\n" " \"chainSupply\": { (object) information about the total supply\n"
" \"monitored\": xx, (boolean) true if the total supply is being monitored\n" " \"monitored\": xx, (boolean) true if the total supply is being monitored\n"
" \"chainValue\": xxxxxx, (numeric, optional) total chain supply\n" " \"chainValue\": xxxxxx, (numeric, optional) total chain supply after this block, in " + CURRENCY_UNIT + "\n"
" \"chainValueZat\": xxxxxx, (numeric, optional) total chain supply in satoshis\n" " \"chainValueZat\": xxxxxx, (numeric, optional) total chain supply after this block, in " + MINOR_CURRENCY_UNIT + "\n"
" }\n" " }\n"
" \"valuePools\": [ (array) information about each value pool\n" " \"valuePools\": [ (array) information about each value pool\n"
" {\n" " {\n"
" \"id\": \"xxxx\", (string) name of the pool\n" " \"id\": \"xxxx\", (string) name of the pool\n"
" \"monitored\": xx, (boolean) true if the pool is being monitored\n" " \"monitored\": xx, (boolean) true if the pool is being monitored\n"
" \"chainValue\": xxxxxx, (numeric, optional) total amount in the pool\n" " \"chainValue\": xxxxxx, (numeric, optional) total amount in the pool, in " + CURRENCY_UNIT + "\n"
" \"chainValueZat\": xxxxxx, (numeric, optional) total amount in the pool in satoshis\n" " \"chainValueZat\": xxxxxx, (numeric, optional) total amount in the pool, in " + MINOR_CURRENCY_UNIT + "\n"
" }, ...\n" " }, ...\n"
" ]\n" " ]\n"
" \"softforks\": [ (array) status of softforks in progress\n" " \"softforks\": [ (array) status of softforks in progress\n"