zcash-hackworks
/
zcash-patched-for-explorer
mirror of https://github.com/zcash-hackworks/zcash-patched-for-explorer.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

throw an exception rather than returning false when building invalid transactions

This commit is contained in:
Eirik Ogilvie-Wigley 2018-10-31 10:15:37 -06:00
parent d55d88707c
commit c297558169
6 changed files with 40 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/gtest/test_transaction_builder.cpp
View File

@ -4,6 +4,7 @@
#include "key_io.h"
#include "main.h"
#include "pubkey.h"
#include "rpc/protocol.h"
#include "transaction_builder.h"
#include "zcash/Address.hpp"
@ -67,9 +68,10 @@ TEST(TransactionBuilder, Invoke)
// Create a Sapling-only transaction
// 0.0004 z-ZEC in, 0.00025 z-ZEC out, 0.0001 t-ZEC fee, 0.00005 z-ZEC change
auto builder2 = TransactionBuilder(consensusParams, 2);
ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note, anchor, witness));
builder2.AddSaplingSpend(expsk, note, anchor, witness);
// Check that trying to add a different anchor fails
ASSERT_FALSE(builder2.AddSaplingSpend(expsk, note, uint256(), witness));
// TODO: the following check can be split out in to another test
ASSERT_THROW(builder2.AddSaplingSpend(expsk, note, uint256(), witness), UniValue);
builder2.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx2 = builder2.Build().GetTxOrThrow();
@ -106,7 +108,7 @@ TEST(TransactionBuilder, RejectsInvalidTransparentOutput)
// Default CTxDestination type is an invalid address
CTxDestination taddr;
auto builder = TransactionBuilder(consensusParams, 1);
EXPECT_FALSE(builder.AddTransparentOutput(taddr, 50));
ASSERT_THROW(builder.AddTransparentOutput(taddr, 50), UniValue);
}
TEST(TransactionBuilder, RejectsInvalidTransparentChangeAddress)
@ -117,7 +119,7 @@ TEST(TransactionBuilder, RejectsInvalidTransparentChangeAddress)
// Default CTxDestination type is an invalid address
CTxDestination taddr;
auto builder = TransactionBuilder(consensusParams, 1);
EXPECT_FALSE(builder.SendChangeTo(taddr));
ASSERT_THROW(builder.SendChangeTo(taddr), UniValue);
}
TEST(TransactionBuilder, FailsWithNegativeChange)
@ -158,12 +160,12 @@ TEST(TransactionBuilder, FailsWithNegativeChange)
// Fail if there is only a transparent output
// 0.0005 t-ZEC out, 0.0001 t-ZEC fee
builder = TransactionBuilder(consensusParams, 1, &keystore);
EXPECT_TRUE(builder.AddTransparentOutput(taddr, 50000));
builder.AddTransparentOutput(taddr, 50000);
EXPECT_EQ("Change cannot be negative", builder.Build().GetError());
// Fails if there is insufficient input
// 0.0005 t-ZEC out, 0.0001 t-ZEC fee, 0.00059999 z-ZEC in
EXPECT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
EXPECT_EQ("Change cannot be negative", builder.Build().GetError());
// Succeeds if there is sufficient input
@ -219,7 +221,7 @@ TEST(TransactionBuilder, ChangeOutput)
{
auto builder = TransactionBuilder(consensusParams, 1, &keystore);
builder.AddTransparentInput(COutPoint(), scriptPubKey, 25000);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
auto tx = builder.Build().GetTxOrThrow();
EXPECT_EQ(tx.vin.size(), 1);
@ -249,7 +251,7 @@ TEST(TransactionBuilder, ChangeOutput)
{
auto builder = TransactionBuilder(consensusParams, 1, &keystore);
builder.AddTransparentInput(COutPoint(), scriptPubKey, 25000);
ASSERT_TRUE(builder.SendChangeTo(taddr));
builder.SendChangeTo(taddr);
auto tx = builder.Build().GetTxOrThrow();
EXPECT_EQ(tx.vin.size(), 1);
@ -290,7 +292,7 @@ TEST(TransactionBuilder, SetFee)
// Default fee
{
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx = builder.Build().GetTxOrThrow();
@ -305,7 +307,7 @@ TEST(TransactionBuilder, SetFee)
// Configured fee
{
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 25000, {});
builder.SetFee(20000);
auto tx = builder.Build().GetTxOrThrow();

22
src/transaction_builder.cpp
View File

@ -54,7 +54,7 @@ TransactionBuilder::TransactionBuilder(
mtx = CreateNewContextualCMutableTransaction(consensusParams, nHeight);
}
bool TransactionBuilder::AddSaplingSpend(
void TransactionBuilder::AddSaplingSpend(
libzcash::SaplingExpandedSpendingKey expsk,
libzcash::SaplingNote note,
uint256 anchor,
@ -66,15 +66,12 @@ bool TransactionBuilder::AddSaplingSpend(
}
// Consistency check: all anchors must equal the first one
if (!spends.empty()) {
if (spends[0].anchor != anchor) {
return false;
}
if (spends.size() > 0 && spends[0].anchor != anchor) {
throw JSONRPCError(RPC_WALLET_ERROR, "Anchor does not match previously-added Sapling spends.");
}
spends.emplace_back(expsk, note, anchor, witness);
mtx.valueBalance += note.value();
return true;
}
void TransactionBuilder::AddSaplingOutput(
@ -103,16 +100,15 @@ void TransactionBuilder::AddTransparentInput(COutPoint utxo, CScript scriptPubKe
tIns.emplace_back(scriptPubKey, value);
}
bool TransactionBuilder::AddTransparentOutput(CTxDestination& to, CAmount value)
void TransactionBuilder::AddTransparentOutput(CTxDestination& to, CAmount value)
{
if (!IsValidDestination(to)) {
return false;
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid output address, not a valid taddr.");
}
CScript scriptPubKey = GetScriptForDestination(to);
CTxOut out(value, scriptPubKey);
mtx.vout.push_back(out);
return true;
}
void TransactionBuilder::SetFee(CAmount fee)
@ -126,16 +122,14 @@ void TransactionBuilder::SendChangeTo(libzcash::SaplingPaymentAddress changeAddr
tChangeAddr = boost::none;
}
bool TransactionBuilder::SendChangeTo(CTxDestination& changeAddr)
void TransactionBuilder::SendChangeTo(CTxDestination& changeAddr)
{
if (!IsValidDestination(changeAddr)) {
return false;
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid change address, not a valid taddr.");
}
tChangeAddr = changeAddr;
zChangeAddr = boost::none;
return true;
}
TransactionBuilderResult TransactionBuilder::Build()
@ -167,7 +161,7 @@ TransactionBuilderResult TransactionBuilder::Build()
AddSaplingOutput(zChangeAddr->first, zChangeAddr->second, change);
} else if (tChangeAddr) {
// tChangeAddr has already been validated.
assert(AddTransparentOutput(tChangeAddr.value(), change));
AddTransparentOutput(tChangeAddr.value(), change);
} else if (!spends.empty()) {
auto fvk = spends[0].expsk.full_viewing_key();
auto note = spends[0].note;

8
src/transaction_builder.h
View File

@ -88,9 +88,9 @@ public:
void SetFee(CAmount fee);
// Returns false if the anchor does not match the anchor used by
// Throws if the anchor does not match the anchor used by
// previously-added Sapling spends.
bool AddSaplingSpend(
void AddSaplingSpend(
libzcash::SaplingExpandedSpendingKey expsk,
libzcash::SaplingNote note,
uint256 anchor,
@ -105,11 +105,11 @@ public:
// Assumes that the value correctly corresponds to the provided UTXO.
void AddTransparentInput(COutPoint utxo, CScript scriptPubKey, CAmount value);
bool AddTransparentOutput(CTxDestination& to, CAmount value);
void AddTransparentOutput(CTxDestination& to, CAmount value);
void SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, uint256 ovk);
bool SendChangeTo(CTxDestination& changeAddr);
void SendChangeTo(CTxDestination& changeAddr);
TransactionBuilderResult Build();
};

6
src/wallet/asyncrpcoperation_mergetoaddress.cpp
View File

@ -338,13 +338,11 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
if (!witnesses[i]) {
throw JSONRPCError(RPC_WALLET_ERROR, "Missing witness for Sapling note");
}
assert(builder_.AddSaplingSpend(expsks[i], saplingNotes[i], anchor, witnesses[i].get()));
builder_.AddSaplingSpend(expsks[i], saplingNotes[i], anchor, witnesses[i].get());
}
if (isToTaddr_) {
if (!builder_.AddTransparentOutput(toTaddr_, sendAmount)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid output address, not a valid taddr.");
}
builder_.AddTransparentOutput(toTaddr_, sendAmount);
} else {
std::string zaddr = std::get<0>(recipient_);
std::string memo = std::get<1>(recipient_);

8
src/wallet/asyncrpcoperation_sendmany.cpp
View File

@ -416,7 +416,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
}
CTxDestination changeAddr = vchPubKey.GetID();
assert(builder_.SendChangeTo(changeAddr));
builder_.SendChangeTo(changeAddr);
}
// Select Sapling notes
@ -445,7 +445,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
if (!witnesses[i]) {
throw JSONRPCError(RPC_WALLET_ERROR, "Missing witness for Sapling note");
}
assert(builder_.AddSaplingSpend(expsk, notes[i], anchor, witnesses[i].get()));
builder_.AddSaplingSpend(expsk, notes[i], anchor, witnesses[i].get());
}
// Add Sapling outputs
@ -469,9 +469,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
auto amount = std::get<1>(r);
auto address = DecodeDestination(outputAddress);
if (!builder_.AddTransparentOutput(address, amount)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid output address, not a valid taddr.");
}
builder_.AddTransparentOutput(address, amount);
}
// Build the transaction

22
src/wallet/gtest/test_wallet.cpp
View File

@ -383,7 +383,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) {
uint256 nullifier = nf.get();
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 50000, {});
builder.SetFee(0);
auto tx = builder.Build().GetTxOrThrow();
@ -501,7 +501,7 @@ TEST(WalletTests, FindMySaplingNotes) {
// Generate transaction
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx = builder.Build().GetTxOrThrow();
@ -639,7 +639,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) {
// Generate tx to create output note B
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 35000, {});
auto tx = builder.Build().GetTxOrThrow();
CWalletTx wtx {&wallet, tx};
@ -693,13 +693,13 @@ TEST(WalletTests, GetConflictedSaplingNotes) {
// Create transaction to spend note B
auto builder2 = TransactionBuilder(consensusParams, 2);
ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness));
builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness);
builder2.AddSaplingOutput(fvk.ovk, pk, 20000, {});
auto tx2 = builder2.Build().GetTxOrThrow();
// Create conflicting transaction which also spends note B
auto builder3 = TransactionBuilder(consensusParams, 2);
ASSERT_TRUE(builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness));
builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness);
builder3.AddSaplingOutput(fvk.ovk, pk, 19999, {});
auto tx3 = builder3.Build().GetTxOrThrow();
@ -798,7 +798,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) {
// Generate transaction
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx = builder.Build().GetTxOrThrow();
@ -893,7 +893,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) {
// Generate transaction
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx = builder.Build().GetTxOrThrow();
@ -1027,7 +1027,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) {
// Generate transaction, which sends funds to note B
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx = builder.Build().GetTxOrThrow();
@ -1097,7 +1097,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) {
// Create transaction to spend note B
auto builder2 = TransactionBuilder(consensusParams, 2);
ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness));
builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness);
builder2.AddSaplingOutput(fvk.ovk, pk, 12500, {});
auto tx2 = builder2.Build().GetTxOrThrow();
EXPECT_EQ(tx2.vin.size(), 0);
@ -1725,7 +1725,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) {
// Generate transaction
auto builder = TransactionBuilder(consensusParams, 1);
ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness));
builder.AddSaplingSpend(expsk, note, anchor, witness);
builder.AddSaplingOutput(fvk.ovk, pk2, 25000, {});
auto tx = builder.Build().GetTxOrThrow();
@ -1928,7 +1928,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) {
// Create a Sapling-only transaction
// 0.0004 z-ZEC in, 0.00025 z-ZEC out, 0.0001 t-ZEC fee, 0.00005 z-ZEC change
auto builder2 = TransactionBuilder(consensusParams, 2);
ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note, anchor, witness));
builder2.AddSaplingSpend(expsk, note, anchor, witness);
builder2.AddSaplingOutput(fvk.ovk, pk, 25000, {});
auto tx2 = builder2.Build().GetTxOrThrow();