Skip to content

Commit 80054f7

Browse files
random-zebrarandom-zebra
committed
[Refactoring] Initialize txbuilder inside sapling_operation
Save a wallet reference when initializing sapling_operation, and pass it to the txbuilder (so we are sure that the keystore used to sign the keys belongs to the wallet used to retrieve the inputs).
1 parent 74a1592 commit 80054f7

File tree

6 files changed

+30
-20
lines changed

6 files changed

+30
-20
lines changed

‎src/qt/walletmodel.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,7 @@ OperationResult WalletModel::PrepareShieldedTransaction(WalletModelTransaction*
607607
if (!opResult) return opResult;
608608

609609
// Create the operation
610-
TransactionBuilder txBuilder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, wallet);
611-
SaplingOperation operation(txBuilder);
610+
SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, wallet);
612611
auto operationResult = operation.setRecipients(recipients)
613612
->setTransparentKeyChange(modelTransaction->getPossibleKeyChange())
614613
->setSelectTransparentCoins(fromTransparent)

‎src/sapling/sapling_operation.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,18 @@ struct TxValues
1919
CAmount target{0};
2020
};
2121

22+
SaplingOperation::SaplingOperation(const Consensus::Params& consensusParams, int nHeight, CWallet* _wallet) :
23+
wallet(_wallet),
24+
txBuilder(consensusParams, nHeight, _wallet)
25+
{
26+
assert (wallet != nullptr);
27+
};
28+
29+
SaplingOperation::~SaplingOperation()
30+
{
31+
delete tkeyChange;
32+
}
33+
2234
OperationResult SaplingOperation::checkTxValues(TxValues& txValues, bool isFromtAddress, bool isFromShielded)
2335
{
2436
assert(!isFromtAddress || txValues.shieldedInTotal == 0);

‎src/sapling/sapling_operation.h‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,8 @@ class FromAddress {
8181

8282
class SaplingOperation {
8383
public:
84-
explicit SaplingOperation(const Consensus::Params& consensusParams, int chainHeight) : txBuilder(consensusParams, chainHeight) {};
85-
explicit SaplingOperation(TransactionBuilder& _builder) : txBuilder(_builder) {};
86-
87-
~SaplingOperation() { delete tkeyChange; }
84+
explicit SaplingOperation(const Consensus::Params& consensusParams, int nHeight, CWallet* _wallet);
85+
~SaplingOperation();
8886

8987
OperationResult build();
9088
OperationResult send(std::string& retTxHash);
@@ -107,6 +105,13 @@ class SaplingOperation {
107105
CTransactionRef getFinalTxRef() { return finalTx; }
108106

109107
private:
108+
/*
109+
* Cannot be nullptr. A pointer to the wallet, used to retrieve the inputs to spend, the keys to create the outputs,
110+
* sapling notes and nullifiers, as well as to commit transactions.
111+
* The same keystore is passed to the transaction builder in order to produce the required signatures.
112+
*/
113+
CWallet* wallet;
114+
110115
FromAddress fromAddress;
111116
// In case of no addressFrom filter selected, it will accept any utxo in the wallet as input.
112117
bool selectFromtaddrs{false};

‎src/test/librust/sapling_rpc_wallet_tests.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
333333
// there are no utxos to spend
334334
{
335335
std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, COIN, "DEADBEEF") };
336-
SaplingOperation operation(consensusParams, 1);
336+
SaplingOperation operation(consensusParams, 1, pwalletMain);
337337
operation.setFromAddress(taddr1);
338338
auto res = operation.setRecipients(recipients)->buildAndSend(ret);
339339
BOOST_CHECK(!res);
@@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
343343
// minconf cannot be zero when sending from zaddr
344344
{
345345
std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, COIN, "DEADBEEF") };
346-
SaplingOperation operation(consensusParams, 1);
346+
SaplingOperation operation(consensusParams, 1, pwalletMain);
347347
operation.setFromAddress(zaddr1);
348348
auto res = operation.setRecipients(recipients)->setMinDepth(0)->buildAndSend(ret);
349349
BOOST_CHECK(!res);
@@ -353,7 +353,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
353353
// there are no unspent notes to spend
354354
{
355355
std::vector<SendManyRecipient> recipients = { SendManyRecipient(taddr1, COIN) };
356-
SaplingOperation operation(consensusParams, 1);
356+
SaplingOperation operation(consensusParams, 1, pwalletMain);
357357
operation.setFromAddress(zaddr1);
358358
auto res = operation.setRecipients(recipients)->buildAndSend(ret);
359359
BOOST_CHECK(!res);
@@ -438,19 +438,16 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
438438
pwalletMain->BlockConnected(std::make_shared<CBlock>(block), mi->second, vtxConflicted);
439439
BOOST_CHECK_MESSAGE(pwalletMain->GetAvailableBalance() > 0, "tx not confirmed");
440440

441-
// Context that shieldsendmany requires
442-
auto builder = TransactionBuilder(consensusParams, nextBlockHeight, pwalletMain);
443-
444441
std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD") };
445-
SaplingOperation operation(builder);
442+
SaplingOperation operation(consensusParams, nextBlockHeight, pwalletMain);
446443
operation.setFromAddress(taddr);
447444
BOOST_CHECK(operation.setRecipients(recipients)
448445
->setMinDepth(0)
449446
->build());
450447

451448
// try from auto-selected transparent address
452449
std::vector<SendManyRecipient> recipients2 = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD") };
453-
SaplingOperation operation2(builder);
450+
SaplingOperation operation2(consensusParams, nextBlockHeight, pwalletMain);
454451
BOOST_CHECK(operation2.setSelectTransparentCoins(true)
455452
->setRecipients(recipients2)
456453
->setMinDepth(0)

‎src/wallet/rpcwallet.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,8 +1168,7 @@ UniValue CreateColdStakeDelegation(const UniValue& params, CTransactionRef& txNe
11681168
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling not active yet");
11691169
}
11701170
std::vector<SendManyRecipient> recipients = {SendManyRecipient(ownerKey, *stakeKey, nValue)};
1171-
TransactionBuilder txBuilder = TransactionBuilder(consensus, nextBlockHeight, pwalletMain);
1172-
SaplingOperation operation(txBuilder);
1171+
SaplingOperation operation(consensus, nextBlockHeight, pwalletMain);
11731172
OperationResult res = operation.setSelectShieldedCoins(true)
11741173
->setRecipients(recipients)
11751174
->build();
@@ -1520,8 +1519,7 @@ static SaplingOperation CreateShieldedTransaction(const JSONRPCRequest& request)
15201519
EnsureWalletIsUnlocked();
15211520
LOCK2(cs_main, pwalletMain->cs_wallet);
15221521
int nextBlockHeight = chainActive.Height() + 1;
1523-
TransactionBuilder txBuilder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, pwalletMain);
1524-
SaplingOperation operation(txBuilder);
1522+
SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, pwalletMain);
15251523

15261524
// Param 0: source of funds. Can either be a valid address, sapling address,
15271525
// or the string "from_transparent"|"from_trans_cold"|"from_shield"

‎src/wallet/test/wallet_sapling_transactions_validations_tests.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ SaplingOperation createOperationAndBuildTx(std::vector<SendManyRecipient> recipi
4343
bool selectTransparentCoins)
4444
{
4545
// Create the operation
46-
TransactionBuilder txBuilder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, pwalletMain);
47-
SaplingOperation operation(txBuilder);
46+
SaplingOperation operation(Params().GetConsensus(), nextBlockHeight, pwalletMain);
4847
auto operationResult = operation.setRecipients(recipients)
4948
->setSelectTransparentCoins(selectTransparentCoins)
5049
->setSelectShieldedCoins(!selectTransparentCoins)

0 commit comments

Comments
 (0)