Skip to content

Commit 2b3f633

Browse files
committed
Safely wrap a C callback in silentpayments_recipient_scan_outputs
1 parent 36b2c6b commit 2b3f633

File tree

1 file changed

+59
-8
lines changed

1 file changed

+59
-8
lines changed

src/silentpayments.rs

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -398,17 +398,17 @@ impl fmt::Display for OutputScanError {
398398
}
399399

400400
/// Scan for Silent Payment transaction outputs.
401-
pub fn silentpayments_recipient_scan_outputs<C: Verification, L>(
401+
pub fn silentpayments_recipient_scan_outputs<
402+
C: Verification,
403+
F: FnMut(&[u8; 33]) -> Option<[u8; 32]>,
404+
>(
402405
secp: &Secp256k1<C>,
403406
tx_outputs: &[&XOnlyPublicKey],
404407
recipient_scan_key: &SecretKey,
405408
public_data: &SilentpaymentsPublicData,
406409
recipient_spend_pubkey: &PublicKey,
407-
label_lookup: SilentpaymentsLabelLookupFunction,
408-
label_context: L,
409-
) -> Result<Vec<SilentpaymentsFoundOutput>, OutputScanError>
410-
{
411-
410+
label_lookup: Option<F>,
411+
) -> Result<Vec<SilentpaymentsFoundOutput>, OutputScanError> {
412412
let cx = secp.ctx().as_ptr();
413413

414414
let n_tx_outputs = tx_outputs.len();
@@ -418,8 +418,59 @@ pub fn silentpayments_recipient_scan_outputs<C: Verification, L>(
418418

419419
let mut n_found_outputs: usize = 0;
420420

421-
let res = unsafe {
421+
type Context<F> = (F, [u8; 32]);
422+
// must live for as long as the c function `secp256k1_silentpayments_recipient_scan_outputs`
423+
let mut context: Context<F>;
424+
let (label_lookup, label_context): (SilentpaymentsLabelLookupFunction, _) =
425+
if let Some(label_lookup) = label_lookup {
426+
unsafe extern "C" fn callback<F: FnMut(&[u8; 33]) -> Option<[u8; 32]>>(
427+
label33: *const u8,
428+
label_context: *const c_void,
429+
) -> *const u8 {
430+
let label33 = unsafe { &*label33.cast::<[u8; 33]>() };
431+
// `.cast_mut()` requires slightly higher (1.65) msrv :(, using `as` instead.
432+
let (f, storage) =
433+
unsafe { &mut *(label_context as *mut c_void).cast::<Context<F>>() };
434+
435+
// `catch_unwind` is needed on Rust < 1.81 to prevent unwinding across an ffi
436+
// boundary, which is undefined behavior. When the user supplied function panics,
437+
// we abort the process. This behavior is consistent with Rust >= 1.81.
438+
match std::panic::catch_unwind(core::panic::AssertUnwindSafe(|| f(label33))) {
439+
Ok(Some(tweak)) => {
440+
// We can't return a pointer to `tweak` as that lives in this function's
441+
// (the callback) stack frame, `storage`, on the other hand, remains valid
442+
// for the duration of secp256k1_silentpayments_recipient_scan_outputs.
443+
*storage = tweak;
444+
storage.as_ptr()
445+
}
446+
Ok(None) => core::ptr::null(),
447+
Err(_) => {
448+
std::process::abort();
449+
}
450+
}
451+
}
452+
453+
context = (label_lookup, [0; 32]);
422454

455+
(callback::<F>, &mut context as *mut Context<F> as *const c_void)
456+
} else {
457+
// TODO we actually want a null pointer, but secp256k1-sys does not allow that (why?).
458+
// A noop is equivalent to null here, but secp256k1 has to do a bit of extra work.
459+
unsafe extern "C" fn noop(
460+
_label33: *const u8,
461+
_label_context: *const c_void,
462+
) -> *const u8 {
463+
core::ptr::null()
464+
}
465+
466+
// TODO `label_context` may be null iff `label_lookup` is null, I requested to remove
467+
// that requirement.
468+
// See https://github.com/bitcoin-core/secp256k1/pull/1519#discussion_r1748595895
469+
(noop, 1usize as *const c_void)
470+
// (core::ptr::null(), core::ptr::null())
471+
};
472+
473+
let res = unsafe {
423474
let ffi_tx_outputs: &[*const ffi::XOnlyPublicKey] = transmute::<&[&XOnlyPublicKey], &[*const ffi::XOnlyPublicKey]>(tx_outputs);
424475

425476
secp256k1_silentpayments_recipient_scan_outputs(
@@ -432,7 +483,7 @@ pub fn silentpayments_recipient_scan_outputs<C: Verification, L>(
432483
public_data.as_c_ptr(),
433484
recipient_spend_pubkey.as_c_ptr(),
434485
label_lookup,
435-
&label_context as *const L as *const c_void,
486+
label_context,
436487
)
437488
};
438489

0 commit comments

Comments
 (0)