Store only signatures for presigned transactions, not the whole PSBT

[darosior]

Instead of storing the PSBTs in DB and querying/updating them, we should only store the signatures and create the PSBTs on the fly. If i recall correctly, it was among other things meant as an optimization to query the presigned transactions.

I think it was premature, and in addition to a big cleanup (both in code and storage) it would prevent races when updating the in-DB psbt. See for instance this monstrosity which is moreover currently buggy (fixed in #366):

revaultd/src/database/actions.rs

Lines 660 to 737 in 3ab3c6a

	// Merge the partial sigs of two transactions of the same type into the first one
	//
	// Returns true if this made the transaction "valid" (fully signed).
	fn revault_txs_merge_sigs<T, S>(tx_a: &mut T, tx_b: &T, secp: &secp256k1::Secp256k1<S>) -> bool
	where
	T: RevaultTransaction,
	S: secp256k1::Verification,
	{
	for (pubkey, sig) in &tx_b.psbt().inputs[0].partial_sigs {
	let sig = secp256k1::Signature::from_der(&sig[..sig.len() - 1]).expect("From DB");
	tx_a.add_signature(0, pubkey.key, sig, secp)
	.expect("From an in-DB PSBT");
	}
	tx_a.is_finalizable(secp)
	}
	// Merge the signatures for two transactions into the first one
	//
	// The two transaction MUST be of the same type.
	fn db_txs_merge_sigs(
	tx_a: &mut DbTransaction,
	tx_b: &DbTransaction,
	secp: &secp256k1::Secp256k1<impl secp256k1::Verification>,
	) -> bool {
	assert_eq!(tx_a.tx_type, tx_b.tx_type);
	match tx_a.psbt {
	RevaultTx::Unvault(ref mut tx_a) => {
	revault_txs_merge_sigs(tx_a, tx_b.psbt.unwrap_unvault(), secp)
	}
	RevaultTx::Cancel(ref mut tx_a) => {
	revault_txs_merge_sigs(tx_a, tx_b.psbt.unwrap_cancel(), secp)
	}
	RevaultTx::Emergency(ref mut tx_a) => {
	revault_txs_merge_sigs(tx_a, tx_b.psbt.unwrap_emer(), secp)
	}
	RevaultTx::UnvaultEmergency(ref mut tx_a) => {
	revault_txs_merge_sigs(tx_a, tx_b.psbt.unwrap_unvault_emer(), secp)
	}
	}
	}
	/// Update the transactions of a given vault with the signatures of the given transactions.
	///
	/// The provided transactions MUST be valid, there signatures aren't checked.
	pub fn db_update_presigned_txs(
	db_path: &Path,
	db_vault: &DbVault,
	transactions: Vec<DbTransaction>,
	secp: &secp256k1::Secp256k1<impl secp256k1::Verification>,
	) -> Result<(), DatabaseError> {
	db_exec(db_path, move |db_tx| {
	for mut transaction in transactions {
	// Merge the transaction with the in-db ones, in case another thread modified
	// it under our feet.
	let db_transaction: DbTransaction = db_tx
	.prepare("SELECT * FROM presigned_transactions WHERE id = (?1)")?
	.query(params![transaction.id])?
	.next()?
	// Note this can happen if another thread removed them.
	.ok_or_else(|| {
	DatabaseError(format!(
	"Transaction with id '{}' (vault id '{}') not found in db",
	transaction.id, db_vault.id
	))
	})?
	.try_into()?;
	let is_fully_signed = db_txs_merge_sigs(&mut transaction, &db_transaction, secp);
	db_tx.execute(
	"UPDATE presigned_transactions SET psbt = (?1), fullysigned = (?2) WHERE id = (?3)",
	params![transaction.psbt.ser(), is_fully_signed, transaction.id],
	)?;
	}
	Ok(())
	})
	}

Another instance where we need to handle possible races when merging the in-DB psbts:

revaultd/src/commands/mod.rs

Lines 556 to 600 in 3ab3c6a

	// Then add them to the PSBTs in database. Take care to update the vault
	// status if all signatures were given via the RPC.
	let rev_txs = vec![cancel_db_tx, emer_db_tx, unvault_emer_db_tx];
	db_update_presigned_txs(&db_path, &db_vault, rev_txs.clone(), secp_ctx)
	.expect("The database must be available");
	db_mark_securing_vault(&db_path, db_vault.id).expect("The database must be available");
	// Now, check whether this made all revocation transactions fully signed
	let emer_tx = db_emer_transaction(&db_path, db_vault.id)
	.expect("Database must be available")
	.ok_or(CommandError::Race)?;
	let cancel_tx = db_cancel_transaction(&db_path, db_vault.id)
	.expect("Database must be available")
	.ok_or(CommandError::Race)?;
	let unemer_tx = db_unvault_emer_transaction(&db_path, db_vault.id)
	.expect("Database must be available")
	.ok_or(CommandError::Race)?;
	let all_rev_fully_signed = emer_tx
	.psbt
	.unwrap_emer()
	.is_finalizable(&revaultd.secp_ctx)
	&& cancel_tx
	.psbt
	.unwrap_cancel()
	.is_finalizable(&revaultd.secp_ctx)
	&& unemer_tx
	.psbt
	.unwrap_unvault_emer()
	.is_finalizable(&revaultd.secp_ctx);
	// If it did, share their signatures with our watchtowers
	if all_rev_fully_signed {
	if let Some(ref watchtowers) = revaultd.watchtowers {
	wts_share_rev_signatures(
	&revaultd.noise_secret,
	&watchtowers,
	db_vault.deposit_outpoint,
	db_vault.derivation_index,
	&emer_tx,
	&cancel_tx,
	&unemer_tx,
	)?;
	}
	}
	db_update_vault_status(&db_path, &db_vault).expect("The database must be available");

[darosior]

I wonder if listpresignedtransactions is an useful API at all.

[darosior]

When we do that, let's keep #369 in mind.