Latest Zcash bug fixes

This commit is contained in:
miketout
2018-10-14 15:09:50 -07:00
6 changed files with 184 additions and 15 deletions

View File

@@ -4,6 +4,16 @@ release-notes at release time)
Notable changes
===============
Enabled Sapling features for mainnet
------------------------------------
This release adds significant support for Sapling to the wallet and RPC interface. Sapling will activate at block 419200, which is expected to be mined on the 28th of October 2018. Users running v2.0.0 nodes (which are consensus-compatible with Sapling) will follow the network upgrade, but must upgrade to v2.0.1 in order to send Sapling shielded transactions.
Minimum Difficulty Blocks allowed on testnet
--------------------------------------------
Sapling activated on testnet at block 280000. Users running v2.0.1 nodes no longer need to specify `-experimentalfeatures` and `-developersapling` to use Sapling functionality on testnet. Users running v2.0.0 nodes should upgrade to v2.0.1 which introduces a consensus rule change to allow minimum difficulty blocks to be mined from block 299188, thereby splitting the chain.
[Pull request](https://github.com/zcash/zcash/pull/3559)
Hierarchical Deterministic Key Generation for Sapling
-----------------------------------------------------
All Sapling addresses will use hierarchical deterministic key generation
@@ -17,3 +27,9 @@ Regular backups are still necessary, however, in order to ensure that
transparent and Sprout addresses are not lost.
[Pull request](https://github.com/zcash/zcash/pull/3492), [ZIP 32](https://github.com/zcash/zips/blob/master/zip-0032.mediawiki)
Fix Signing Raw Transactions with Unsynced Offline Nodes
--------------------------------------------------------
With v2.0.0, in `signrawtransaction` the consensus branch ID (which is used to construct the transaction) was estimated using the chain height. With v2.0.1 this has been improved by also considering the `APPROX_RELEASE_HEIGHT` of the release, and a new parameter to allow the caller to manually override the consensus branch ID that zcashd will use.
[Pull request](https://github.com/zcash/zcash/pull/3520)

View File

@@ -16,15 +16,16 @@ class WalletPersistenceTest (BitcoinTestFramework):
def setup_chain(self):
print("Initializing test directory " + self.options.tmpdir)
initialize_chain_clean(self.options.tmpdir, 2)
initialize_chain_clean(self.options.tmpdir, 3)
def setup_network(self, split=False):
self.nodes = start_nodes(2, self.options.tmpdir,
self.nodes = start_nodes(3, self.options.tmpdir,
extra_args=[[
'-nuparams=5ba81b19:100', # Overwinter
'-nuparams=76b809bb:201', # Sapling
]] * 2)
]] * 3)
connect_nodes_bi(self.nodes,0,1)
connect_nodes_bi(self.nodes,1,2)
self.is_network_split=False
self.sync_all()
@@ -97,5 +98,34 @@ class WalletPersistenceTest (BitcoinTestFramework):
assert_equal(self.nodes[0].z_getbalance(sapling_addr), Decimal('5'))
assert_equal(self.nodes[1].z_getbalance(dest_addr), Decimal('15'))
# Verify importing a spending key will update and persist the nullifiers and witnesses correctly
sk0 = self.nodes[0].z_exportkey(sapling_addr)
self.nodes[2].z_importkey(sk0, "yes")
assert_equal(self.nodes[2].z_getbalance(sapling_addr), Decimal('5'))
# Restart the nodes
stop_nodes(self.nodes)
wait_bitcoinds()
self.setup_network()
# Verify nullifiers persisted correctly by checking balance
# Prior to PR #3590, there will be an error as spent notes are considered unspent:
# Assertion failed: expected: <25.00000000> but was: <5>
assert_equal(self.nodes[2].z_getbalance(sapling_addr), Decimal('5'))
# Verity witnesses persisted correctly by sending shielded funds
recipients = []
recipients.append({"address": dest_addr, "amount": Decimal('1')})
myopid = self.nodes[2].z_sendmany(sapling_addr, recipients, 1, 0)
wait_and_assert_operationid_status(self.nodes[2], myopid)
self.sync_all()
self.nodes[0].generate(1)
self.sync_all()
# Verify balances
assert_equal(self.nodes[2].z_getbalance(sapling_addr), Decimal('4'))
assert_equal(self.nodes[1].z_getbalance(dest_addr), Decimal('16'))
if __name__ == '__main__':
WalletPersistenceTest().main()

View File

@@ -4,6 +4,7 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
from test_framework.test_framework import BitcoinTestFramework
from test_framework.authproxy import JSONRPCException
from test_framework.util import (
assert_equal,
start_nodes,
@@ -18,17 +19,57 @@ class WalletSaplingTest(BitcoinTestFramework):
def setup_nodes(self):
return start_nodes(4, self.options.tmpdir, [[
'-nuparams=5ba81b19:201', # Overwinter
'-nuparams=76b809bb:201', # Sapling
'-nuparams=76b809bb:203', # Sapling
'-experimentalfeatures', '-zmergetoaddress',
]] * 4)
def run_test(self):
# Sanity-check the test harness
assert_equal(self.nodes[0].getblockcount(), 200)
# Activate Sapling
# Activate Overwinter
self.nodes[2].generate(1)
self.sync_all()
# Verify RPCs disallow Sapling value transfer if Sapling is not active
tmp_taddr = self.nodes[3].getnewaddress()
tmp_zaddr = self.nodes[3].z_getnewaddress('sapling')
try:
recipients = []
recipients.append({"address": tmp_zaddr, "amount": Decimal('20')})
self.nodes[3].z_sendmany(tmp_taddr, recipients, 1, 0)
raise AssertionError("Should have thrown an exception")
except JSONRPCException as e:
assert_equal("Invalid parameter, Sapling has not activated", e.error['message'])
try:
recipients = []
recipients.append({"address": tmp_taddr, "amount": Decimal('20')})
self.nodes[3].z_sendmany(tmp_zaddr, recipients, 1, 0)
raise AssertionError("Should have thrown an exception")
except JSONRPCException as e:
assert_equal("Invalid parameter, Sapling has not activated", e.error['message'])
try:
self.nodes[3].z_shieldcoinbase(tmp_taddr, tmp_zaddr)
raise AssertionError("Should have thrown an exception")
except JSONRPCException as e:
assert_equal("Invalid parameter, Sapling has not activated", e.error['message'])
# Verify z_mergetoaddress RPC does not support Sapling yet
try:
self.nodes[3].z_mergetoaddress([tmp_taddr], tmp_zaddr)
raise AssertionError("Should have thrown an exception")
except JSONRPCException as e:
assert_equal("Invalid parameter, Sapling is not supported yet by z_mergetoadress", e.error['message'])
try:
self.nodes[3].z_mergetoaddress([tmp_zaddr], tmp_taddr)
raise AssertionError("Should have thrown an exception")
except JSONRPCException as e:
assert_equal("Invalid parameter, Sapling is not supported yet by z_mergetoadress", e.error['message'])
# Activate Sapling
self.nodes[2].generate(2)
self.sync_all()
taddr0 = self.nodes[0].getnewaddress()
# Skip over the address containing node 1's coinbase
self.nodes[1].getnewaddress()
@@ -144,5 +185,18 @@ class WalletSaplingTest(BitcoinTestFramework):
self.nodes[2].z_importkey(sk1, "yes")
assert_equal(self.nodes[2].z_getbalance(saplingAddr1), Decimal('5'))
# Make sure we get a useful error when trying to send to both sprout and sapling
node4_sproutaddr = self.nodes[3].z_getnewaddress('sprout')
node4_saplingaddr = self.nodes[3].z_getnewaddress('sapling')
try:
self.nodes[1].z_sendmany(
taddr1,
[{'address': node4_sproutaddr, 'amount': 2.5}, {'address': node4_saplingaddr, 'amount': 2.4999}],
1, 0.0001
)
raise AssertionError("Should have thrown an exception")
except JSONRPCException as e:
assert_equal("Cannot send to both Sprout and Sapling addresses using z_sendmany", e.error['message'])
if __name__ == '__main__':
WalletSaplingTest().main()

View File

@@ -201,15 +201,6 @@ public:
fMineBlocksOnDemand = false;
fTestnetToBeDeprecatedFieldRPC = false;
// LogPrintf(">>>>>>>> ac_name = %u\n",GetArg("-ac_name","").c_str());
// if ( GetArg("-ac_name","").c_str()[0] != 0 )
// {
// }
// else
// {
// }
if ( pthread_create((pthread_t *)malloc(sizeof(pthread_t)),NULL,chainparams_commandline,(void *)&consensus) != 0 )
{

View File

@@ -4037,6 +4037,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
std::vector<SendManyRecipient> zaddrRecipients;
CAmount nTotalOut = 0;
bool containsSproutOutput = false;
bool containsSaplingOutput = false;
for (const UniValue& o : outputs.getValues()) {
if (!o.isObject())
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected object");
@@ -4060,6 +4063,16 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
bool toSprout = !toSapling;
noSproutAddrs = noSproutAddrs && toSapling;
containsSproutOutput |= toSprout;
containsSaplingOutput |= toSapling;
// Sending to both Sprout and Sapling is currently unsupported using z_sendmany
if (containsSproutOutput && containsSaplingOutput) {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
"Cannot send to both Sprout and Sapling addresses using z_sendmany");
}
// If we are sending from a shielded address, all recipient
// shielded addresses must be of the same type.
if (fromSprout && toSapling) {
@@ -4134,6 +4147,13 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
}
}
// If Sapling is not active, do not allow sending from or sending to Sapling addresses.
if (!NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) {
if (fromSapling || containsSaplingOutput) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated");
}
}
// As a sanity check, estimate and verify that the size of the transaction will be valid.
// Depending on the input notes, the actual tx size may turn out to be larger and perhaps invalid.
size_t txsize = 0;
@@ -4323,6 +4343,19 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
max_tx_size = MAX_TX_SIZE_BEFORE_SAPLING;
}
// If Sapling is not active, do not allow sending to a Sapling address.
if (!NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) {
auto res = DecodePaymentAddress(destaddress);
if (IsValidPaymentAddress(res)) {
bool toSapling = boost::get<libzcash::SaplingPaymentAddress>(&res) != nullptr;
if (toSapling) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated");
}
} else {
throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress );
}
}
// Prepare to get coinbase utxos
std::vector<ShieldCoinbaseUTXO> inputs;
CAmount shieldedValue = 0;
@@ -4539,6 +4572,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
std::set<std::string> setAddress;
// Sources
bool containsSaplingZaddrSource = false;
for (const UniValue& o : addresses.getValues()) {
if (!o.isStr())
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected string");
@@ -4564,6 +4598,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
if (!(useAny || useAnyNote)) {
zaddrs.insert(zaddr);
}
// Check if z-addr is Sapling
bool isSapling = boost::get<libzcash::SaplingPaymentAddress>(&zaddr) != nullptr;
containsSaplingZaddrSource |= isSapling;
} else {
throw JSONRPCError(
RPC_INVALID_PARAMETER,
@@ -4580,10 +4617,19 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
// Validate the destination address
auto destaddress = params[1].get_str();
bool isToZaddr = false;
bool isToSaplingZaddr = false;
CTxDestination taddr = DecodeDestination(destaddress);
if (!IsValidDestination(taddr)) {
if (IsValidPaymentAddressString(destaddress, branchId)) {
isToZaddr = true;
// Is this a Sapling address?
auto res = DecodePaymentAddress(destaddress);
if (IsValidPaymentAddress(res)) {
isToSaplingZaddr = boost::get<libzcash::SaplingPaymentAddress>(&res) != nullptr;
} else {
throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress );
}
} else {
throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress );
}
@@ -4639,6 +4685,19 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
max_tx_size = MAX_TX_SIZE_BEFORE_SAPLING;
}
// This RPC does not support Sapling yet.
if (isToSaplingZaddr || containsSaplingZaddrSource) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling is not supported yet by z_mergetoadress");
}
// If this RPC does support Sapling...
// If Sapling is not active, do not allow sending from or sending to Sapling addresses.
if (!NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) {
if (isToSaplingZaddr || containsSaplingZaddrSource) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated");
}
}
// Prepare to get UTXOs and notes
std::vector<MergeToAddressInputUTXO> utxoInputs;
std::vector<MergeToAddressInputNote> noteInputs;

View File

@@ -1439,6 +1439,7 @@ int32_t CWallet::VerusStakeTransaction(CBlock *pBlock, CMutableTransaction &txNe
// !! DISABLE THIS FOR RELEASE: THIS MAKES A CHEAT TRANSACTION FOR EVERY STAKE FOR TESTING
CMutableTransaction cheat;
cheat = CMutableTransaction(txNew);
printf("TESTING ONLY: THIS SHOULD NOT BE ENABLED FOR RELEASE - MAKING CHEAT TRANSACTION FOR TESTING");
cheat.vout[1].scriptPubKey << OP_RETURN
<< CStakeParams(pSrcIndex->GetHeight(), tipindex->GetHeight() + 1, pSrcIndex->GetBlockHash(), pk).AsVector();
// !! DOWN TO HERE
@@ -2719,6 +2720,9 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
const CChainParams& chainParams = Params();
CBlockIndex* pindex = pindexStart;
std::vector<uint256> myTxHashes;
{
LOCK2(cs_main, cs_wallet);
@@ -2739,8 +2743,10 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
ReadBlockFromDisk(block, pindex,1);
BOOST_FOREACH(CTransaction& tx, block.vtx)
{
if (AddToWalletIfInvolvingMe(tx, &block, fUpdate))
if (AddToWalletIfInvolvingMe(tx, &block, fUpdate)) {
myTxHashes.push_back(tx.GetHash());
ret++;
}
}
SproutMerkleTree sproutTree;
@@ -2762,6 +2768,19 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
LogPrintf("Still rescanning. At block %d. Progress=%f\n", pindex->GetHeight(), Checkpoints::GuessVerificationProgress(chainParams.Checkpoints(), pindex));
}
}
// After rescanning, persist Sapling note data that might have changed, e.g. nullifiers.
// Do not flush the wallet here for performance reasons.
CWalletDB walletdb(strWalletFile, "r+", false);
for (auto hash : myTxHashes) {
CWalletTx wtx = mapWallet[hash];
if (!wtx.mapSaplingNoteData.empty()) {
if (!wtx.WriteToDisk(&walletdb)) {
LogPrintf("Rescanning... WriteToDisk failed to update Sapling note data for: %s\n", hash.ToString());
}
}
}
ShowProgress(_("Rescanning..."), 100); // hide progress dialog in GUI
}
return ret;