Merge pull request #6408 from sellout/wallet_tx_builder/lock-notes

Lock notes (except Orchard) in wallet_tx_builder
This commit is contained in:
Kris Nuttycombe 2023-03-31 18:05:06 -06:00 committed by GitHub
commit 2d456afebe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 94 additions and 36 deletions

View File

@ -84,7 +84,7 @@ void AsyncRPCOperation_sendmany::main() {
std::optional<uint256> txid;
try {
txid = main_impl();
txid = main_impl(*pwalletMain);
} catch (const UniValue& objError) {
int code = find_value(objError, "code").get_int();
std::string message = find_value(objError, "message").get_str();
@ -137,11 +137,11 @@ void AsyncRPCOperation_sendmany::main() {
// 4. #3615 There is no padding of inputs or outputs, which may leak information.
//
// At least #4 differs from the Rust transaction builder.
uint256 AsyncRPCOperation_sendmany::main_impl() {
auto spendable = builder_.FindAllSpendableInputs(*pwalletMain, ztxoSelector_, mindepth_);
uint256 AsyncRPCOperation_sendmany::main_impl(CWallet& wallet) {
auto spendable = builder_.FindAllSpendableInputs(wallet, ztxoSelector_, mindepth_);
auto preparedTx = builder_.PrepareTransaction(
*pwalletMain,
wallet,
ztxoSelector_,
spendable,
recipients_,
@ -156,37 +156,43 @@ uint256 AsyncRPCOperation_sendmany::main_impl() {
ThrowInputSelectionError(err, ztxoSelector_, strategy_);
},
[&](const TransactionEffects& effects) {
const auto& spendable = effects.GetSpendable();
const auto& payments = effects.GetPayments();
spendable.LogInputs(getId());
try {
const auto& spendable = effects.GetSpendable();
const auto& payments = effects.GetPayments();
spendable.LogInputs(getId());
LogPrint("zrpcunsafe", "%s: spending %s to send %s with fee %s\n", getId(),
FormatMoney(payments.Total()),
FormatMoney(spendable.Total()),
FormatMoney(effects.GetFee()));
LogPrint("zrpc", "%s: total transparent input: %s (to choose from)\n", getId(),
FormatMoney(spendable.GetTransparentTotal()));
LogPrint("zrpcunsafe", "%s: total shielded input: %s (to choose from)\n", getId(),
FormatMoney(spendable.GetSaplingTotal() + spendable.GetOrchardTotal()));
LogPrint("zrpc", "%s: total transparent output: %s\n", getId(),
FormatMoney(payments.GetTransparentTotal()));
LogPrint("zrpcunsafe", "%s: total shielded Sapling output: %s\n", getId(),
FormatMoney(payments.GetSaplingTotal()));
LogPrint("zrpcunsafe", "%s: total shielded Orchard output: %s\n", getId(),
FormatMoney(payments.GetOrchardTotal()));
LogPrint("zrpc", "%s: fee: %s\n", getId(), FormatMoney(effects.GetFee()));
LogPrint("zrpcunsafe", "%s: spending %s to send %s with fee %s\n", getId(),
FormatMoney(payments.Total()),
FormatMoney(spendable.Total()),
FormatMoney(effects.GetFee()));
LogPrint("zrpc", "%s: total transparent input: %s (to choose from)\n", getId(),
FormatMoney(spendable.GetTransparentTotal()));
LogPrint("zrpcunsafe", "%s: total shielded input: %s (to choose from)\n", getId(),
FormatMoney(spendable.GetSaplingTotal() + spendable.GetOrchardTotal()));
LogPrint("zrpc", "%s: total transparent output: %s\n", getId(),
FormatMoney(payments.GetTransparentTotal()));
LogPrint("zrpcunsafe", "%s: total shielded Sapling output: %s\n", getId(),
FormatMoney(payments.GetSaplingTotal()));
LogPrint("zrpcunsafe", "%s: total shielded Orchard output: %s\n", getId(),
FormatMoney(payments.GetOrchardTotal()));
LogPrint("zrpc", "%s: fee: %s\n", getId(), FormatMoney(effects.GetFee()));
auto buildResult = effects.ApproveAndBuild(
Params().GetConsensus(),
*pwalletMain,
chainActive,
strategy_);
auto tx = buildResult.GetTxOrThrow();
auto buildResult = effects.ApproveAndBuild(
Params().GetConsensus(),
wallet,
chainActive,
strategy_);
auto tx = buildResult.GetTxOrThrow();
UniValue sendResult = SendTransaction(tx, payments.GetResolvedPayments(), std::nullopt, testmode);
set_result(sendResult);
UniValue sendResult = SendTransaction(tx, payments.GetResolvedPayments(), std::nullopt, testmode);
set_result(sendResult);
txid = tx.GetHash();
txid = tx.GetHash();
effects.UnlockSpendable(wallet);
} catch (...) {
effects.UnlockSpendable(wallet);
throw;
}
}
}, preparedTx);

View File

@ -64,7 +64,7 @@ private:
CAmount fee_;
UniValue contextinfo_; // optional data to include in return value from getStatus()
uint256 main_impl();
uint256 main_impl(CWallet& wallet);
};
// To test private methods, a friend class can act as a proxy
@ -74,8 +74,8 @@ public:
TEST_FRIEND_AsyncRPCOperation_sendmany(std::shared_ptr<AsyncRPCOperation_sendmany> ptr) : delegate(ptr) {}
uint256 main_impl() {
return delegate->main_impl();
uint256 main_impl(CWallet& wallet) {
return delegate->main_impl(wallet);
}
void set_state(OperationStatus state) {

View File

@ -147,7 +147,7 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
auto ovks = SelectOVKs(wallet, selector, spendable);
return TransactionEffects(
auto effects = TransactionEffects(
anchorConfirmations,
spendable,
resolvedPayments,
@ -156,6 +156,8 @@ PrepareTransactionResult WalletTxBuilder::PrepareTransaction(
ovks.first,
ovks.second,
anchorHeight);
effects.LockSpendable(wallet);
return effects;
}
Payments InputSelection::GetPayments() const {
@ -646,3 +648,35 @@ TransactionBuilderResult TransactionEffects::ApproveAndBuild(
// Build the transaction
return builder.Build();
}
// TODO: Lock Orchard notes (#6226)
void TransactionEffects::LockSpendable(CWallet& wallet) const
{
LOCK2(cs_main, wallet.cs_wallet);
for (auto utxo : spendable.utxos) {
COutPoint outpt(utxo.tx->GetHash(), utxo.i);
wallet.LockCoin(outpt);
}
for (auto note : spendable.sproutNoteEntries) {
wallet.LockNote(note.jsop);
}
for (auto note : spendable.saplingNoteEntries) {
wallet.LockNote(note.op);
}
}
// TODO: Unlock Orchard notes (#6226)
void TransactionEffects::UnlockSpendable(CWallet& wallet) const
{
LOCK2(cs_main, wallet.cs_wallet);
for (auto utxo : spendable.utxos) {
COutPoint outpt(utxo.tx->GetHash(), utxo.i);
wallet.UnlockCoin(outpt);
}
for (auto note : spendable.sproutNoteEntries) {
wallet.UnlockNote(note.jsop);
}
for (auto note : spendable.saplingNoteEntries) {
wallet.UnlockNote(note.op);
}
}

View File

@ -48,7 +48,9 @@ public:
PaymentAddress address,
CAmount amount,
std::optional<Memo> memo) :
address(address), amount(amount), memo(memo) {}
address(address), amount(amount), memo(memo) {
assert(amount >= 0);
}
const PaymentAddress& GetAddress() const {
return address;
@ -185,6 +187,22 @@ public:
return spendable;
}
/**
* This should be called upon creating `TransactionEffects`, it locks exactly the notes that
* will be spent in the built transaction.
*/
void LockSpendable(CWallet& wallet) const;
/**
* This should be called when we are finished with the transaction (whether it succeeds or
* fails).
*
* TODO: This currently needs to be called while the `TransactionEffects` exists. In future, it
* would be useful to keep these notes locked until we have confirmation that the tx is on
* the chain or not.
*/
void UnlockSpendable(CWallet& wallet) const;
const Payments& GetPayments() const {
return payments;
}