Skip to content

Commit

Permalink
Spark runaway exceptions (#1344)
Browse files Browse the repository at this point in the history
* Avoid catch(...) when possible

* Add wrappers around major sigma/lelantus/spark calls to catch exceptions

* Catch exception thrown from CheckLelantusJMintTransaction

* Bug fix

* Fixed tests

* Disable failing elysium test
  • Loading branch information
psolstice authored Nov 21, 2023
1 parent 1a2d024 commit 62fc651
Show file tree
Hide file tree
Showing 26 changed files with 147 additions and 110 deletions.
2 changes: 1 addition & 1 deletion qa/pull-tester/rpc-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
'lelantus_spend_gettransaction.py',
'elysium_create_denomination.py',
'elysium_property_creation_fee.py',
'elysium_sendmint.py',
# 'elysium_sendmint.py',
'elysium_sendmint_wallet_encryption.py',
'elysium_sendspend.py',
'elysium_sendspend_wallet_encryption.py',
Expand Down
6 changes: 3 additions & 3 deletions src/batchproof_container.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ void BatchProofContainer::batch_sigma() {
try {
if (!sigmaVerifier.batch_verify(anonymity_set, serials, fPadding, setSizes, proofs))
return false;
} catch (...) {
} catch (const std::exception &) {
return false;
}
return true;
Expand Down Expand Up @@ -316,7 +316,7 @@ void BatchProofContainer::batch_lelantus() {
try {
if (!sigmaVerifier.batchverify(anonymity_set, challenges, serials, setSizes, proofs))
return false;
} catch (...) {
} catch (const std::exception &) {
return false;
}
return true;
Expand Down Expand Up @@ -431,7 +431,7 @@ void BatchProofContainer::batch_spark() {
bool passed;
try {
passed = spark::SpendTransaction::verify(params, sparkTransactions, cover_sets);
} catch (...) {
} catch (const std::exception &) {
passed = false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/bip47/account.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ bool CAccountReceiver::acceptMaskedPayload(std::vector<unsigned char> const & ma
std::unique_ptr<lelantus::JoinSplit> jsplit;
try {
jsplit = lelantus::ParseLelantusJoinSplit(tx);
}catch (...) {
}catch (const std::exception &) {
return false;
}
if (!jsplit)
Expand Down
2 changes: 1 addition & 1 deletion src/bip47/bip47utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ GroupElement GeFromPubkey(CPubKey const & pubKey)
serializedGe.push_back(0x0);
try {
result.deserialize(&serializedGe[0]);
} catch (...) {
} catch (const std::exception &) {
result = GroupElement();
}
return result;
Expand Down
6 changes: 3 additions & 3 deletions src/chainparams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ class CMainParams : public CChainParams {
GroupElement coin;
try {
coin.deserialize(ParseHex(str).data());
} catch (...) {
} catch (const std::exception &) {
continue;
}
consensus.lelantusBlacklist.insert(coin);
Expand All @@ -435,7 +435,7 @@ class CMainParams : public CChainParams {
GroupElement coin;
try {
coin.deserialize(ParseHex(str).data());
} catch (...) {
} catch (const std::exception &) {
continue;
}
consensus.sigmaBlacklist.insert(coin);
Expand Down Expand Up @@ -728,7 +728,7 @@ class CTestNetParams : public CChainParams {
GroupElement coin;
try {
coin.deserialize(ParseHex(str).data());
} catch (...) {
} catch (const std::exception &) {
continue;
}
consensus.lelantusBlacklist.insert(coin);
Expand Down
2 changes: 1 addition & 1 deletion src/elysium/elysium.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2329,7 +2329,7 @@ int elysium::WalletTxBuilder(
case InputMode::SIGMA:
try {
if (!pwalletMain->CommitSigmaTransaction(wtxNew, sigmaSelected, sigmaChanges)) return MP_ERR_COMMIT_TX;
} catch (...) {
} catch (const std::exception &) {
return MP_ERR_COMMIT_TX;
}
break;
Expand Down
2 changes: 1 addition & 1 deletion src/elysium/rpctx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1689,7 +1689,7 @@ UniValue elysium_sendmint(const JSONRPCRequest& request)
if (result != 0) {
throw JSONRPCError(result, error_str(result));
}
} catch (...) {
} catch (const std::exception &) {
for (auto& id : ids) {
wallet->DeleteUnconfirmedSigmaMint(id);
}
Expand Down
2 changes: 1 addition & 1 deletion src/elysium/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ SigmaPrivateKey Wallet::GetKey(const SigmaMint &mint)
// Try all mint wallets
try {
return mintWalletV1.GeneratePrivateKey(mint.seedId);
} catch (...) {
} catch (const std::exception &) {
return mintWalletV0.GeneratePrivateKey(mint.seedId);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/hdmint/tracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ bool CHDMintTracker::IsMempoolSpendOurs(const std::set<uint256>& setMempool, con
uint32_t pubcoinId;
try {
std::tie(spend, pubcoinId) = sigma::ParseSigmaSpend(txin);
} catch (...) {
} catch (const std::exception &) {
return false;
}

Expand All @@ -560,7 +560,7 @@ bool CHDMintTracker::IsMempoolSpendOurs(const std::set<uint256>& setMempool, con
std::unique_ptr<lelantus::JoinSplit> joinsplit;
try {
joinsplit = lelantus::ParseLelantusJoinSplit(tx);
} catch (...) {
} catch (const std::exception &) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/hdmint/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,7 @@ bool CHDMintWallet::TxOutToPublicCoin(const CTxOut& txout, sigma::PublicCoin& pu
secp_primitives::GroupElement publicSigma;
try {
publicSigma.deserialize(&coin_serialised[0]);
} catch (...) {
} catch (const std::exception &) {
return state.DoS(100, error("TxOutToPublicCoin : deserialize failed"));
}

Expand Down
51 changes: 32 additions & 19 deletions src/lelantus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ bool CheckLelantusJoinSplitTransaction(
REJECT_MALFORMED,
"CheckLelantusJoinSplitTransaction: invalid joinsplit transaction");
}
catch (...) {
catch (const std::exception &) {
return state.DoS(100,
false,
REJECT_MALFORMED,
Expand Down Expand Up @@ -444,8 +444,13 @@ bool CheckLelantusJoinSplitTransaction(

for (const CTxOut &txout : tx.vout) {
if (!txout.scriptPubKey.empty() && txout.scriptPubKey.IsLelantusJMint()) {
if (!CheckLelantusJMintTransaction(txout, state, hashTx, fStatefulSigmaCheck, Cout, lelantusTxInfo))
return false;
try {
if (!CheckLelantusJMintTransaction(txout, state, hashTx, fStatefulSigmaCheck, Cout, lelantusTxInfo))
return false;
}
catch (const std::exception &x) {
return state.Error(x.what());
}
} else if(txout.scriptPubKey.IsLelantusMint()) {
return false; //putting regular mints at JoinSplit transactions is not allowed
} else {
Expand Down Expand Up @@ -767,8 +772,13 @@ bool CheckLelantusTransaction(
if (allowLelantus && !isVerifyDB) {
for (const CTxOut &txout : tx.vout) {
if (!txout.scriptPubKey.empty() && txout.scriptPubKey.IsLelantusMint()) {
if (!CheckLelantusMintTransaction(txout, state, hashTx, fStatefulSigmaCheck, lelantusTxInfo))
return false;
try {
if (!CheckLelantusMintTransaction(txout, state, hashTx, fStatefulSigmaCheck, lelantusTxInfo))
return false;
}
catch (const std::exception &x) {
return state.Error(x.what());
}
}
}
}
Expand All @@ -789,10 +799,15 @@ bool CheckLelantusTransaction(
}

if (!isVerifyDB) {
if (!CheckLelantusJoinSplitTransaction(
tx, state, hashTx, isVerifyDB, nHeight, realHeight,
isCheckWallet, fStatefulSigmaCheck, sigmaTxInfo, lelantusTxInfo)) {
return false;
try {
if (!CheckLelantusJoinSplitTransaction(
tx, state, hashTx, isVerifyDB, nHeight, realHeight,
isCheckWallet, fStatefulSigmaCheck, sigmaTxInfo, lelantusTxInfo)) {
return false;
}
}
catch (const std::exception &x) {
return state.Error(x.what());
}
}
}
Expand All @@ -815,7 +830,7 @@ void RemoveLelantusJoinSplitReferencingBlock(CTxMemPool& pool, CBlockIndex* bloc
try {
joinsplit = ParseLelantusJoinSplit(tx);
}
catch (...) {
catch (const std::exception &) {
txn_to_remove.push_back(tx);
break;
}
Expand Down Expand Up @@ -854,7 +869,7 @@ std::vector<Scalar> GetLelantusJoinSplitSerialNumbers(const CTransaction &tx, co
try {
return ParseLelantusJoinSplit(tx)->getCoinSerialNumbers();
}
catch (...) {
catch (const std::exception &) {
return std::vector<Scalar>();
}
}
Expand All @@ -866,7 +881,7 @@ std::vector<uint32_t> GetLelantusJoinSplitIds(const CTransaction &tx, const CTxI
try {
return ParseLelantusJoinSplit(tx)->getCoinGroupIds();
}
catch (...) {
catch (const std::exception &) {
return std::vector<uint32_t>();
}
}
Expand Down Expand Up @@ -1006,7 +1021,7 @@ bool GetOutPointFromBlock(COutPoint& outPoint, const GroupElement &pubCoinValue,
try {
ParseLelantusMintScript(txout.scriptPubKey, txPubCoinValue);
}
catch (...) {
catch (const std::exception &) {
continue;
}
if(pubCoinValue==txPubCoinValue){
Expand Down Expand Up @@ -1131,13 +1146,11 @@ void CLelantusState::Containers::RemoveMint(lelantus::PublicCoin const & pubCoin
}

void CLelantusState::Containers::AddSpend(Scalar const & serial, int coinGroupId) {
if (!mintMetaInfo.count(coinGroupId)) {
throw std::invalid_argument("group id doesn't exist");
if (mintMetaInfo.count(coinGroupId) > 0) {
usedCoinSerials[serial] = coinGroupId;
spendMetaInfo[coinGroupId] += 1;
CheckSurgeCondition();
}

usedCoinSerials[serial] = coinGroupId;
spendMetaInfo[coinGroupId] += 1;
CheckSurgeCondition();
}

void CLelantusState::Containers::RemoveSpend(Scalar const & serial) {
Expand Down
2 changes: 1 addition & 1 deletion src/liblelantus/lelantus_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ void LelantusProver::generate_sigma_proofs(
parallelTasks.emplace_back(threadPool.PostTask([&]() {
try {
prover.sigma_commit(commits, index, rA_i, rB_i, rC_i, rD_i, a_i, Tk_i, Pk_i, Yk_i, sigma_i, proof);
} catch (...) {
} catch (const std::exception &) {
return false;
}
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/libspark/coin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ IdentifiedCoinData Coin::identify(const IncomingViewKey& incoming_view_key) {
// Decrypt recipient data
CDataStream stream = AEAD::decrypt_and_verify(this->K*incoming_view_key.get_s1(), "Mint coin data", this->r_);
stream >> r;
} catch (...) {
} catch (const std::exception &) {
throw std::runtime_error("Unable to identify coin");
}

Expand All @@ -142,7 +142,7 @@ IdentifiedCoinData Coin::identify(const IncomingViewKey& incoming_view_key) {
// Decrypt recipient data
CDataStream stream = AEAD::decrypt_and_verify(this->K*incoming_view_key.get_s1(), "Spend coin data", this->r_);
stream >> r;
} catch (...) {
} catch (const std::exception &) {
throw std::runtime_error("Unable to identify coin");
}

Expand Down
4 changes: 2 additions & 2 deletions src/libspark/hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Scalar Hash::finalize_scalar() {
EVP_MD_CTX_free(state_finalize);

return candidate;
} catch (...) {
} catch (const std::exception &) {
counter++;
}
}
Expand Down Expand Up @@ -144,7 +144,7 @@ GroupElement Hash::finalize_group() {
EVP_MD_CTX_free(state_finalize);

return candidate;
} catch (...) {
} catch (const std::exception &) {
counter++;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libspark/transcript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ Scalar Transcript::challenge(const std::string label) {
EVP_MD_CTX_free(state_finalize);

return candidate;
} catch (...) {
} catch (const std::exception &) {
counter++;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libspark/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ GroupElement SparkUtils::hash_generator(const std::string label) {
EVP_MD_CTX_free(state_finalize);

return candidate;
} catch (...) {
} catch (const std::exception &) {
counter++;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/qt/transactiondesc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco
try {
nTxFee = lelantus::ParseLelantusJoinSplit(*wtx.tx)->getFee();
}
catch (...) {
catch (const std::exception &) {
//do nothing
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/qt/transactionrecord.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
if (isAllJoinSplitFromMe && wtx.tx->vin.size() > 0) {
try {
nTxFee = lelantus::ParseLelantusJoinSplit(*wtx.tx)->getFee();
} catch (...) {
} catch (const std::exception &) {
// do nothing
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry)
try {
jsplit = lelantus::ParseLelantusJoinSplit(tx);
}
catch (...) {
catch (const std::exception &) {
continue;
}
in.push_back(Pair("nFees", ValueFromAmount(jsplit->getFee())));
Expand All @@ -143,7 +143,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry)
try {
sparkSpend = std::make_unique<spark::SpendTransaction>(spark::ParseSparkSpend(tx));
}
catch (...) {
catch (const std::exception &) {
continue;
}
in.push_back(Pair("nFees", ValueFromAmount(sparkSpend->getFee())));
Expand Down
24 changes: 17 additions & 7 deletions src/sigma.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,13 @@ bool CheckSigmaTransaction(
if (allowSigma) {
for (const CTxOut &txout : tx.vout) {
if (!txout.scriptPubKey.empty() && txout.scriptPubKey.IsSigmaMint()) {
if (!CheckSigmaMintTransaction(txout, state, hashTx, fStatefulSigmaCheck, sigmaTxInfo))
return false;
try {
if (!CheckSigmaMintTransaction(txout, state, hashTx, fStatefulSigmaCheck, sigmaTxInfo))
return false;
}
catch (const std::exception &x) {
return state.Error(x.what());
}
}
}
}
Expand Down Expand Up @@ -508,10 +513,15 @@ bool CheckSigmaTransaction(
// Check vOut
// Only one loop, we checked on the format before entering this case
if (!isVerifyDB) {
if (!CheckSigmaSpendTransaction(
tx, denominations, state, hashTx, isVerifyDB, nHeight, realHeight,
isCheckWallet, fStatefulSigmaCheck, sigmaTxInfo)) {
return false;
try {
if (!CheckSigmaSpendTransaction(
tx, denominations, state, hashTx, isVerifyDB, nHeight, realHeight,
isCheckWallet, fStatefulSigmaCheck, sigmaTxInfo)) {
return false;
}
}
catch (const std::exception &x) {
return state.Error(x.what());
}
}
}
Expand Down Expand Up @@ -665,7 +675,7 @@ bool GetOutPointFromBlock(COutPoint& outPoint, const GroupElement &pubCoinValue,
txout.scriptPubKey.end());
try {
txPubCoinValue.deserialize(&coin_serialised[0]);
} catch (...) {
} catch (const std::exception &) {
return false;
}
if(pubCoinValue==txPubCoinValue){
Expand Down
Loading

0 comments on commit 62fc651

Please sign in to comment.