Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Script][BUG] Fix signature malleability for t inputs in Sapling txes #2064

Merged
merged 7 commits into from
Dec 16, 2020
Merged

[Script][BUG] Fix signature malleability for t inputs in Sapling txes #2064

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99a4379
BUG: SSPKM, guard GetDebit/GetCredit for non-shield v2 txes
random-zebra Dec 15, 2020
f62ac6c
BUG: Guard access to tx.sapData in connectBlock
random-zebra Dec 15, 2020
fcbdd23
Tests: fix order of sapData serialization
random-zebra Dec 15, 2020
9cc790f
Tests: Add trolling exploit demo
random-zebra Dec 15, 2020
2d7f8e2
Script: Properly sign/verify shielded tx with sigversion sapling
random-zebra Dec 15, 2020
cee6ef3
Tests: update sighash json test data for saplingVersion txes
random-zebra Dec 15, 2020
e5b8dba
Tests: Update sapling_malleable-sigs test, now failing
random-zebra Dec 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/sapling/saplingscriptpubkeyman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,9 @@ isminetype SaplingScriptPubKeyMan::IsMine(const CWalletTx& wtx, const SaplingOut

CAmount SaplingScriptPubKeyMan::GetCredit(const CWalletTx& tx, const isminefilter& filter, const bool fUnspent) const
{
if (!tx.IsShieldedTx() || tx.sapData->vShieldedOutput.empty()) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsShieldedTx is asking for vShieldedOutput.empty inside. Would be good if we add a new method hasShieldedOutputs.
(Same for the GetDebit, an hasShieldedSpends)

return 0;
}
CAmount nCredit = 0;
for (int i = 0; i < (int) tx.sapData->vShieldedOutput.size(); ++i) {
SaplingOutPoint op(tx.GetHash(), i);
Expand All @@ -697,6 +700,9 @@ CAmount SaplingScriptPubKeyMan::GetCredit(const CWalletTx& tx, const isminefilte

CAmount SaplingScriptPubKeyMan::GetDebit(const CTransaction& tx, const isminefilter& filter) const
{
if (!tx.IsShieldedTx() || tx.sapData->vShieldedSpend.empty()) {
return 0;
}
CAmount nDebit = 0;
for (const SpendDescription& spend : tx.sapData->vShieldedSpend) {
const auto &it = mapSaplingNullifiersToNotes.find(spend.nullifier);
Expand All @@ -720,7 +726,7 @@ CAmount SaplingScriptPubKeyMan::GetDebit(const CTransaction& tx, const isminefil

CAmount SaplingScriptPubKeyMan::GetShieldedChange(const CWalletTx& wtx) const
{
if (!wtx.isSaplingVersion() || wtx.sapData->vShieldedOutput.empty()) {
if (!wtx.IsShieldedTx() || wtx.sapData->vShieldedOutput.empty()) {
return 0;
}
const uint256& txHash = wtx.GetHash();
Expand Down