Auto merge of #3885 - ebfull:turnstile, r=bitcartel

Reject blocks that violate turnstile

This is an implementation of a consensus rule which marks blocks as invalid if they would lead to a turnstile violation in the Sprout or Shielded value pools. The motivations and deployment details can be found in the [accompanying ZIP draft](https://github.com/zcash/zips/pull/210).

**This PR only introduces the rule for testnet at the moment.**

We achieve the institution of this rule in three ways:

1. Nodes prior to #2795 did not record the "delta" in the Sprout value pool balance as part of the on-disk block index. This was a long time ago, though, and all nodes that are consensus-compatible with the network today have been recording this value for newer blocks. However, the value is absent from older block indexes unless the node has reindexed or synchronized from scratch in the time since. We shouldn't need to require nodes to reindex in order to enforce this consensus rule. We avoid this problem by falling back on a hardcoded Sprout shielded value pool balance in a very recent block.
2. If during `ConnectBlock` we observe that the resulting shielded value pool balance of Sprout or Sapling is negative, we reject the block.
3. During the miner's block assembly process the miner will skip over transactions if adding them to the assembled block might violate the turnstile, since the resulting block would be invalid. This means that theoretical transactions violating the turnstile would still be relayed in the network (and made available in users' memory pools) and so a turnstile violation would have some visibility outside of block relay.

## Smoke Testing

It's really tricky to test the behavior that automatically falls back to hardcoded shielded value pool balances in our architecture because it's very testnet-specific and node-version-specific. However, we can do some smoke tests to see that everything is working.

I modified the serialization of `CDiskBlockIndex` to serialize `boost::none` for `nSproutValue`

```
if ((s.GetType() & SER_DISK) && (nVersion >= SPROUT_VALUE_VERSION)) {
    boost::optional<CAmount> nSproutValueFake = boost::none;
    READWRITE(nSproutValueFake);
}
```

and then began a reindex of my node which I interruped around height 130k on testnet. I then restored the original serialization and resumed the reindex; I have thus _roughly_ simulated a older node "upgrading" to a newer node that records the deltas when processing new blocks. My node showed pool monitoring was disabled, as expected, for Sprout. I confirmed that some blocks following the reindex had nonzero Sprout `valueDelta` from `getblock`, as expected. I finished the reindex, restarted the node, and confirmed that the serialization worked for newer blocks but not older blocks by querying `getblock`, simply as a reassurance.

Finally, I introduced the code in this PR and reloaded the node. The desired behavior (that the chain began to be "monitored" again) worked, and the values were consistent with the hardcoded constant. I then made a payment to a Sprout z-addr from the transparent pool and the pool value increased as expected, as reported by `getblockchaininfo`. I reindexed the node again to exercise the remaining logic and check for turnstile violations throughout the history of testnet; there were none.
This commit is contained in:
Homu 2019-03-19 12:10:17 -07:00
commit 008705d7c1
4 changed files with 140 additions and 0 deletions

View File

@ -354,6 +354,14 @@ public:
715 // total number of tx / (checkpoint block height / (24 * 24))
};
// Hardcoded fallback value for the Sprout shielded value pool balance
// for nodes that have not reindexed since the introduction of monitoring
// in #2795.
nSproutValuePoolCheckpointHeight = 440329;
nSproutValuePoolCheckpointBalance = 40000029096803;
fZIP209Enabled = true;
hashSproutValuePoolCheckpointBlock = uint256S("000a95d08ba5dcbabe881fc6471d11807bcca7df5f1795c99f3ec4580db4279b");
// Founders reward script expects a vector of 2-of-3 multisig addresses
vFoundersRewardAddress = {
"t2UNzUUx8mWBCRYPRezvA363EYXyEpHokyi", "t2N9PH9Wk9xjqYg9iin1Ua3aekJqfAtE543", "t2NGQjYMQhFndDHguvUw4wZdNdsssA6K7x2", "t2ENg7hHVqqs9JwU5cgjvSbxnT2a9USNfhy",

View File

@ -70,6 +70,11 @@ public:
const std::vector<unsigned char>& AlertKey() const { return vAlertPubKey; }
int GetDefaultPort() const { return nDefaultPort; }
CAmount SproutValuePoolCheckpointHeight() const { return nSproutValuePoolCheckpointHeight; }
CAmount SproutValuePoolCheckpointBalance() const { return nSproutValuePoolCheckpointBalance; }
uint256 SproutValuePoolCheckpointBlockHash() const { return hashSproutValuePoolCheckpointBlock; }
bool ZIP209Enabled() const { return fZIP209Enabled; }
const CBlock& GenesisBlock() const { return genesis; }
/** Make miner wait to have peers to avoid wasting work */
bool MiningRequiresPeers() const { return fMiningRequiresPeers; }
@ -125,6 +130,11 @@ protected:
bool fTestnetToBeDeprecatedFieldRPC = false;
CCheckpointData checkpointData;
std::vector<std::string> vFoundersRewardAddress;
CAmount nSproutValuePoolCheckpointHeight = 0;
CAmount nSproutValuePoolCheckpointBalance = 0;
uint256 hashSproutValuePoolCheckpointBlock;
bool fZIP209Enabled = false;
};
/**

View File

@ -2451,6 +2451,35 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
return true;
}
// Reject a block that results in a negative shielded value pool balance.
if (chainparams.ZIP209Enabled()) {
// Sprout
//
// We can expect nChainSproutValue to be valid after the hardcoded
// height, and this will be enforced on all descendant blocks. If
// the node was reindexed then this will be enforced for all blocks.
if (pindex->nChainSproutValue) {
if (*pindex->nChainSproutValue < 0) {
return state.DoS(100, error("ConnectBlock(): turnstile violation in Sprout shielded value pool"),
REJECT_INVALID, "turnstile-violation-sprout-shielded-pool");
}
}
// Sapling
//
// If we've reached ConnectBlock, we have all transactions of
// parents and can expect nChainSaplingValue not to be boost::none.
// However, the miner and mining RPCs may not have populated this
// value and will call `TestBlockValidity`. So, we act
// conditionally.
if (pindex->nChainSaplingValue) {
if (*pindex->nChainSaplingValue < 0) {
return state.DoS(100, error("ConnectBlock(): turnstile violation in Sapling shielded value pool"),
REJECT_INVALID, "turnstile-violation-sapling-shielded-pool");
}
}
}
// Do not allow blocks that contain transactions which 'overwrite' older transactions,
// unless those are already completely spent.
BOOST_FOREACH(const CTransaction& tx, block.vtx) {
@ -3296,9 +3325,46 @@ CBlockIndex* AddToBlockIndex(const CBlockHeader& block)
return pindexNew;
}
void FallbackSproutValuePoolBalance(
CBlockIndex *pindex,
const CChainParams& chainparams
)
{
// We might not want to enable the checkpointing for mainnet
// yet.
if (!chainparams.ZIP209Enabled()) {
return;
}
// Check if the height of this block matches the checkpoint
if (pindex->nHeight == chainparams.SproutValuePoolCheckpointHeight()) {
if (pindex->GetBlockHash() == chainparams.SproutValuePoolCheckpointBlockHash()) {
// Are we monitoring the Sprout pool?
if (!pindex->nChainSproutValue) {
// Apparently not. Introduce the hardcoded value so we monitor for
// this point onwards (assuming the checkpoint is late enough)
pindex->nChainSproutValue = chainparams.SproutValuePoolCheckpointBalance();
} else {
// Apparently we have been. So, we should expect the current
// value to match the hardcoded one.
assert(*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance());
// And we should expect non-none for the delta stored in the block index here,
// or the checkpoint is too early.
assert(pindex->nSproutValue != boost::none);
}
} else {
LogPrintf(
"FallbackSproutValuePoolBalance(): fallback block hash is incorrect, we got %s\n",
pindex->GetBlockHash().ToString()
);
}
}
}
/** Mark a block as having its data received and checked (up to BLOCK_VALID_TRANSACTIONS). */
bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBlockIndex *pindexNew, const CDiskBlockPos& pos)
{
const CChainParams& chainparams = Params();
pindexNew->nTx = block.vtx.size();
pindexNew->nChainTx = 0;
CAmount sproutValue = 0;
@ -3351,6 +3417,10 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl
pindex->nChainSproutValue = pindex->nSproutValue;
pindex->nChainSaplingValue = pindex->nSaplingValue;
}
// Fall back to hardcoded Sprout value pool balance
FallbackSproutValuePoolBalance(pindex, chainparams);
{
LOCK(cs_nBlockSequenceId);
pindex->nSequenceId = nBlockSequenceId++;
@ -4003,6 +4073,7 @@ CBlockIndex * InsertBlockIndex(uint256 hash)
bool static LoadBlockIndexDB()
{
const CChainParams& chainparams = Params();
if (!pblocktree->LoadBlockIndexGuts())
return false;
@ -4048,6 +4119,9 @@ bool static LoadBlockIndexDB()
pindex->nChainSproutValue = pindex->nSproutValue;
pindex->nChainSaplingValue = pindex->nSaplingValue;
}
// Fall back to hardcoded Sprout value pool balance
FallbackSproutValuePoolBalance(pindex, chainparams);
}
// Construct in-memory chain of branch IDs.
// Relies on invariant: a block that does not activate a network upgrade

View File

@ -250,6 +250,28 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
TxPriorityCompare comparer(fSortedByFee);
std::make_heap(vecPriority.begin(), vecPriority.end(), comparer);
// We want to track the value pool, but if the miner gets
// invoked on an old block before the hardcoded fallback
// is active we don't want to trip up any assertions. So,
// we only adhere to the turnstile (as a miner) if we
// actually have all of the information necessary to do
// so.
CAmount sproutValue = 0;
CAmount saplingValue = 0;
bool monitoring_pool_balances = true;
if (chainparams.ZIP209Enabled()) {
if (pindexPrev->nChainSproutValue) {
sproutValue = *pindexPrev->nChainSproutValue;
} else {
monitoring_pool_balances = false;
}
if (pindexPrev->nChainSaplingValue) {
saplingValue = *pindexPrev->nChainSaplingValue;
} else {
monitoring_pool_balances = false;
}
}
while (!vecPriority.empty())
{
// Take highest priority transaction off the priority queue:
@ -305,6 +327,32 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, txdata, Params().GetConsensus(), consensusBranchId))
continue;
if (chainparams.ZIP209Enabled() && monitoring_pool_balances) {
// Does this transaction lead to a turnstile violation?
CAmount sproutValueDummy = sproutValue;
CAmount saplingValueDummy = saplingValue;
saplingValueDummy += -tx.valueBalance;
for (auto js : tx.vjoinsplit) {
sproutValueDummy += js.vpub_old;
sproutValueDummy -= js.vpub_new;
}
if (sproutValueDummy < 0) {
LogPrintf("CreateNewBlock(): tx %s appears to violate Sprout turnstile\n", tx.GetHash().ToString());
continue;
}
if (saplingValueDummy < 0) {
LogPrintf("CreateNewBlock(): tx %s appears to violate Sapling turnstile\n", tx.GetHash().ToString());
continue;
}
sproutValue = sproutValueDummy;
saplingValue = saplingValueDummy;
}
UpdateCoins(tx, view, nHeight);
BOOST_FOREACH(const OutputDescription &outDescription, tx.vShieldedOutput) {