Skip to content

Commit 8acb8f9

Browse files
ekrembalekrembal
authored
Merge branch 'cantina-fixes' into atacan/txsender_cpfp_fix
2 parents f937912 + c617d9a commit 8acb8f9

File tree

5 files changed

+136
-50
lines changed

5 files changed

+136
-50
lines changed

‎core/src/actor.rs‎

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,18 @@ impl Actor {
618618
.map(|s| s.sighash_type())?
619619
.unwrap_or(TapSighashType::Default);
620620

621+
// Common calculations across all spend paths
622+
let signature_id = spt.get_signature_id();
623+
let sig = Self::get_saved_signature(signature_id, signatures);
624+
let sighash = calc_sighash(sighash_type)?;
625+
621626
match spt.get_spend_path() {
622627
SpendPath::ScriptSpend(script_idx) => {
623628
let script = spt
624629
.get_spendable()
625630
.get_scripts()
626631
.get(script_idx)
627632
.ok_or(TxError::NoScriptAtIndex(script_idx))?;
628-
let sig = Self::get_saved_signature(spt.get_signature_id(), signatures);
629633

630634
let sig = sig.map(|sig| taproot::Signature {
631635
signature: sig,
@@ -638,10 +642,19 @@ impl Actor {
638642
let mut witness: Witness = match script.kind() {
639643
Kind::BaseDepositScript(script) => {
640644
match (sig, script.0 == self.xonly_public_key) {
641-
(Some(sig), _) => script.generate_script_inputs(&sig),
645+
(Some(sig), _) => {
646+
Self::verify_signature(
647+
&sig.signature,
648+
sighash,
649+
script.0,
650+
TapTweakData::ScriptPath,
651+
&signature_id,
652+
)?;
653+
script.generate_script_inputs(&sig)
654+
}
642655
(None, true) => {
643656
script.generate_script_inputs(&taproot::Signature {
644-
signature: self.sign(calc_sighash(sighash_type)?),
657+
signature: self.sign(sighash),
645658
sighash_type,
646659
})
647660
}
@@ -652,10 +665,19 @@ impl Actor {
652665
}
653666
Kind::ReplacementDepositScript(script) => {
654667
match (sig, script.0 == self.xonly_public_key) {
655-
(Some(sig), _) => script.generate_script_inputs(&sig),
668+
(Some(sig), _) => {
669+
Self::verify_signature(
670+
&sig.signature,
671+
sighash,
672+
script.0,
673+
TapTweakData::ScriptPath,
674+
&signature_id,
675+
)?;
676+
script.generate_script_inputs(&sig)
677+
}
656678
(None, true) => {
657679
script.generate_script_inputs(&taproot::Signature {
658-
signature: self.sign(calc_sighash(sighash_type)?),
680+
signature: self.sign(sighash),
659681
sighash_type,
660682
})
661683
}
@@ -665,10 +687,19 @@ impl Actor {
665687
}
666688
}
667689
Kind::TimelockScript(script) => match (sig, script.0) {
668-
(Some(sig), Some(_)) => script.generate_script_inputs(Some(&sig)),
690+
(Some(sig), Some(xonly_pk)) => {
691+
Self::verify_signature(
692+
&sig.signature,
693+
sighash,
694+
xonly_pk,
695+
TapTweakData::ScriptPath,
696+
&signature_id,
697+
)?;
698+
script.generate_script_inputs(Some(&sig))
699+
}
669700
(None, Some(xonly_key)) if xonly_key == self.xonly_public_key => script
670701
.generate_script_inputs(Some(&taproot::Signature {
671-
signature: self.sign(calc_sighash(sighash_type)?),
702+
signature: self.sign(sighash),
672703
sighash_type,
673704
})),
674705
(None, Some(_)) => {
@@ -677,10 +708,19 @@ impl Actor {
677708
(_, None) => Witness::new(),
678709
},
679710
Kind::CheckSig(script) => match (sig, script.0 == self.xonly_public_key) {
680-
(Some(sig), _) => script.generate_script_inputs(&sig),
711+
(Some(sig), _) => {
712+
Self::verify_signature(
713+
&sig.signature,
714+
sighash,
715+
script.0,
716+
TapTweakData::ScriptPath,
717+
&signature_id,
718+
)?;
719+
script.generate_script_inputs(&sig)
720+
}
681721

682722
(None, true) => script.generate_script_inputs(&taproot::Signature {
683-
signature: self.sign(calc_sighash(sighash_type)?),
723+
signature: self.sign(sighash),
684724
sighash_type,
685725
}),
686726
(None, false) => return Err(TxError::SignatureNotFound(tx_type).into()),
@@ -701,14 +741,20 @@ impl Actor {
701741
}
702742
SpendPath::KeySpend => {
703743
let xonly_public_key = spendinfo.internal_key();
704-
705-
let sighash = calc_sighash(sighash_type)?;
706-
let sig = Self::get_saved_signature(spt.get_signature_id(), signatures);
707744
let sig = match sig {
708-
Some(sig) => taproot::Signature {
709-
signature: sig,
710-
sighash_type,
711-
},
745+
Some(sig) => {
746+
Self::verify_signature(
747+
&sig,
748+
sighash,
749+
xonly_public_key,
750+
TapTweakData::KeyPath(spendinfo.merkle_root()),
751+
&signature_id,
752+
)?;
753+
taproot::Signature {
754+
signature: sig,
755+
sighash_type,
756+
}
757+
}
712758
None => {
713759
if xonly_public_key == self.xonly_public_key {
714760
taproot::Signature {
@@ -733,6 +779,28 @@ impl Actor {
733779
txhandler.sign_txins(signer)?;
734780
Ok(())
735781
}
782+
783+
/// Verifies a schnorr signature with the given parameters and wraps errors with signature ID context.
784+
fn verify_signature(
785+
sig: &schnorr::Signature,
786+
sighash: TapSighash,
787+
xonly_public_key: XOnlyPublicKey,
788+
tweak_data: TapTweakData,
789+
signature_id: &SignatureId,
790+
) -> Result<(), BridgeError> {
791+
verify_schnorr(
792+
sig,
793+
&Message::from(sighash),
794+
xonly_public_key,
795+
tweak_data,
796+
None,
797+
)
798+
.wrap_err(format!(
799+
"Failed to verify signature from DB for signature {:?} for signer xonly pk {}",
800+
signature_id, xonly_public_key
801+
))
802+
.map_err(Into::into)
803+
}
736804
}
737805

738806
#[cfg(test)]

‎core/src/builder/transaction/sign.rs‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use crate::utils::{Last20Bytes, RbfSigningInfo};
2020
use crate::verifier::Verifier;
2121
use bitcoin::hashes::Hash;
2222
use bitcoin::{BlockHash, OutPoint, Transaction, XOnlyPublicKey};
23+
use eyre::Context;
2324
use rand_chacha::rand_core::SeedableRng;
2425
use rand_chacha::ChaCha12Rng;
2526
use secp256k1::rand::seq::SliceRandom;
@@ -157,7 +158,12 @@ pub async fn create_and_sign_txs(
157158
let mut tweak_cache = TweakCache::default();
158159

159160
for (tx_type, mut txhandler) in txhandlers.into_iter() {
160-
let _ = signer.tx_sign_and_fill_sigs(&mut txhandler, &signatures, Some(&mut tweak_cache));
161+
let _ = signer
162+
.tx_sign_and_fill_sigs(&mut txhandler, &signatures, Some(&mut tweak_cache))
163+
.wrap_err(format!(
164+
"Couldn't sign transaction {:?} in create_and_sign_txs for context {:?}",
165+
tx_type, context,
166+
));
161167

162168
if let TransactionType::OperatorChallengeAck(watchtower_idx) = tx_type {
163169
let path = WinternitzDerivationPath::ChallengeAckHash(
@@ -195,7 +201,7 @@ pub async fn create_and_sign_txs(
195201
signed_txs.push((tx_type, checked_txhandler.get_cached_tx().clone()));
196202
}
197203
Err(e) => {
198-
tracing::trace!(
204+
tracing::debug!(
199205
"Couldn't sign transaction {:?} in create_and_sign_all_txs: {:?}.
200206
This might be normal if the transaction is not needed to be/cannot be signed.",
201207
tx_type,
@@ -369,11 +375,18 @@ where
369375
// do not try to sign unrelated txs
370376
continue;
371377
}
372-
self.signer.tx_sign_and_fill_sigs(
373-
&mut txhandler,
374-
&unspent_kickoff_sigs,
375-
Some(&mut tweak_cache),
376-
)?;
378+
let res = self.signer
379+
.tx_sign_and_fill_sigs(
380+
&mut txhandler,
381+
&unspent_kickoff_sigs,
382+
Some(&mut tweak_cache),
383+
)
384+
.wrap_err(format!(
385+
"Couldn't sign transaction {:?} in create_and_sign_unspent_kickoff_connector_txs for round {:?} and operator {}",
386+
tx_type,
387+
round_idx,
388+
operator_xonly_pk,
389+
));
377390

378391
let checked_txhandler = txhandler.promote();
379392

@@ -383,9 +396,10 @@ where
383396
}
384397
Err(e) => {
385398
tracing::trace!(
386-
"Couldn't sign transaction {:?} in create_and_sign_unspent_kickoff_connector_txs: {:?}",
399+
"Couldn't sign transaction {:?} in create_and_sign_unspent_kickoff_connector_txs: {:?}: {:?}",
387400
tx_type,
388-
e
401+
e,
402+
res.err()
389403
);
390404
}
391405
}

‎core/src/builder/transaction/txhandler.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub struct Unsigned;
5454
impl State for Unsigned {}
5555
impl State for Signed {}
5656
pub type SighashCalculator<'a> =
57-
Box<dyn FnOnce(TapSighashType) -> Result<TapSighash, BridgeError> + 'a>;
57+
Box<dyn Fn(TapSighashType) -> Result<TapSighash, BridgeError> + 'a>;
5858

5959
impl<T: State> TxHandler<T> {
6060
/// Returns a spendable input for the specified output index in this transaction.
@@ -142,7 +142,7 @@ impl<T: State> TxHandler<T> {
142142
fn get_sighash_calculator(
143143
&self,
144144
idx: usize,
145-
) -> impl FnOnce(TapSighashType) -> Result<TapSighash, BridgeError> + '_ {
145+
) -> impl Fn(TapSighashType) -> Result<TapSighash, BridgeError> + '_ {
146146
move |sighash_type: TapSighashType| -> Result<TapSighash, BridgeError> {
147147
match self.txins[idx].get_spend_path() {
148148
SpendPath::KeySpend => self.calculate_pubkey_spend_sighash(idx, sighash_type),

‎core/src/operator.rs‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,7 +1011,8 @@ where
10111011
)?;
10121012

10131013
self.signer
1014-
.tx_sign_and_fill_sigs(&mut first_round_tx, &[], None)?;
1014+
.tx_sign_and_fill_sigs(&mut first_round_tx, &[], None)
1015+
.wrap_err("Failed to sign first round tx")?;
10151016

10161017
self.tx_sender
10171018
.insert_try_to_send(
@@ -1088,18 +1089,18 @@ where
10881089
let mut tweak_cache = TweakCache::default();
10891090

10901091
// sign ready to reimburse tx
1091-
self.signer.tx_sign_and_fill_sigs(
1092-
&mut ready_to_reimburse_txhandler,
1093-
&[],
1094-
Some(&mut tweak_cache),
1095-
)?;
1092+
self.signer
1093+
.tx_sign_and_fill_sigs(
1094+
&mut ready_to_reimburse_txhandler,
1095+
&[],
1096+
Some(&mut tweak_cache),
1097+
)
1098+
.wrap_err("Failed to sign ready to reimburse tx")?;
10961099

10971100
// sign next round tx
1098-
self.signer.tx_sign_and_fill_sigs(
1099-
&mut next_round_txhandler,
1100-
&[],
1101-
Some(&mut tweak_cache),
1102-
)?;
1101+
self.signer
1102+
.tx_sign_and_fill_sigs(&mut next_round_txhandler, &[], Some(&mut tweak_cache))
1103+
.wrap_err("Failed to sign next round tx")?;
11031104

11041105
let current_round_txid = current_round_txhandler.get_cached_tx().compute_txid();
11051106
let ready_to_reimburse_tx = ready_to_reimburse_txhandler.get_cached_tx();
@@ -1163,11 +1164,13 @@ where
11631164
)?;
11641165

11651166
// sign burn unused kickoff connectors tx
1166-
self.signer.tx_sign_and_fill_sigs(
1167-
&mut burn_unspent_kickoff_connectors_tx,
1168-
&[],
1169-
Some(&mut tweak_cache),
1170-
)?;
1167+
self.signer
1168+
.tx_sign_and_fill_sigs(
1169+
&mut burn_unspent_kickoff_connectors_tx,
1170+
&[],
1171+
Some(&mut tweak_cache),
1172+
)
1173+
.wrap_err("Failed to sign burn unused kickoff connectors tx")?;
11711174

11721175
self.tx_sender
11731176
.insert_try_to_send(
@@ -2162,11 +2165,10 @@ where
21622165
)?;
21632166

21642167
// sign burn unused kickoff connectors tx
2165-
self.signer.tx_sign_and_fill_sigs(
2166-
&mut burn_unused_kickoff_connectors_txhandler,
2167-
&[],
2168-
None,
2169-
)?;
2168+
self.signer
2169+
.tx_sign_and_fill_sigs(&mut burn_unused_kickoff_connectors_txhandler, &[], None)
2170+
.wrap_err("Failed to sign burn unused kickoff connectors tx")?;
2171+
21702172
let burn_unused_kickoff_connectors_txhandler =
21712173
burn_unused_kickoff_connectors_txhandler.promote()?;
21722174
Ok(vec![(

‎core/src/verifier.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2425,7 +2425,8 @@ where
24252425
verifier_xonly_pk,
24262426
e
24272427
);
2428-
})?;
2428+
})
2429+
.wrap_err("Failed to sign disprove tx")?;
24292430

24302431
let disprove_tx = disprove_txhandler.get_cached_tx().clone();
24312432

@@ -2712,7 +2713,8 @@ where
27122713
verifier_xonly_pk,
27132714
e
27142715
);
2715-
})?;
2716+
})
2717+
.wrap_err("Failed to sign disprove tx")?;
27162718

27172719
let disprove_tx = disprove_txhandler.get_cached_tx().clone();
27182720

0 commit comments

Comments
 (0)