Skip to content

Commit 0591658

Browse files
OttoAllmendingerllm-git
andcommitted
feat(wasm-utxo): add BCH fork ID support for BCH, XEC, BSV, BTG
Implement fork ID support for Bitcoin Cash-style networks to enable proper signing and verification. This includes: - Use fork ID-aware signing for BCH (0), BTG (79), XEC (0), and BSV (0) - Update miniscript dependency to use forkid-compatible version - Modify PSBT functions to use the appropriate sighash algorithm - Ensure replay protection inputs use correct sighash calculation - Add tests to verify sign→verify roundtrip works correctly Issue: BTC-2656 Co-authored-by: llm-git <[email protected]>
1 parent 42945f4 commit 0591658

File tree

5 files changed

+206
-58
lines changed

5 files changed

+206
-58
lines changed

packages/wasm-utxo/Cargo.lock

Lines changed: 2 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/wasm-utxo/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ all = "warn"
1616
[dependencies]
1717
wasm-bindgen = "0.2"
1818
js-sys = "0.3"
19-
miniscript = { git = "https://github.com/BitGo/rust-miniscript", tag = "miniscript-13.0.0-opdrop" }
19+
miniscript = { git = "https://github.com/BitGo/rust-miniscript", tag = "miniscript-13.0.0-opdrop-forkid" }
2020
bech32 = "0.11"
2121
musig2 = { version = "0.3.1", default-features = false, features = ["k256"] }
2222
getrandom = { version = "0.2", features = ["js"] }

packages/wasm-utxo/cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ base64 = "0.21"
1616
serde = { version = "1.0", features = ["derive"] }
1717
serde_json = "1.0"
1818
num-bigint = "0.4"
19-
bitcoin = "0.32"
19+
bitcoin = { git = "https://github.com/BitGo/rust-bitcoin", tag = "bitcoin-0.32.8-forkid" }
2020
colored = "2.1"
2121
ptree = "0.5"

packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs

Lines changed: 186 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -314,16 +314,24 @@ impl BitGoPsbt {
314314
use miniscript::psbt::PsbtExt;
315315

316316
match self {
317-
BitGoPsbt::BitcoinLike(ref mut psbt, _network) => {
317+
BitGoPsbt::BitcoinLike(ref mut psbt, network) => {
318318
// Use custom bitgo p2trMusig2 input finalization for MuSig2 inputs
319319
if p2tr_musig2_input::Musig2Input::is_musig2_input(&psbt.inputs[input_index]) {
320320
let mut ctx = p2tr_musig2_input::Musig2Context::new(psbt, input_index)
321321
.map_err(|e| e.to_string())?;
322322
ctx.finalize_input(secp).map_err(|e| e.to_string())?;
323323
return Ok(());
324324
}
325-
// other inputs can be finalized using the standard miniscript::psbt::finalize_input
326-
psbt.finalize_inp_mut(secp, input_index)
325+
326+
// Check if this network uses SIGHASH_FORKID (BCH, BTG, XEC, BSV)
327+
let fork_id = match network.mainnet() {
328+
Network::BitcoinCash | Network::Ecash | Network::BitcoinSV => Some(0u32),
329+
Network::BitcoinGold => Some(79u32),
330+
_ => None,
331+
};
332+
333+
// Finalize with fork_id support for FORKID networks
334+
psbt.finalize_inp_mut_with_fork_id(secp, input_index, fork_id)
327335
.map_err(|e| e.to_string())?;
328336
Ok(())
329337
}
@@ -589,19 +597,56 @@ impl BitGoPsbt {
589597
);
590598
}
591599

592-
// Sign the replay protection input with legacy P2SH sighash
593-
let sighash_type = miniscript::bitcoin::sighash::EcdsaSighashType::All;
594-
let cache = SighashCache::new(&psbt.unsigned_tx);
595-
let sighash = cache
596-
.legacy_signature_hash(input_index, redeem_script, sighash_type.to_u32())
597-
.map_err(|e| format!("Failed to compute sighash: {}", e))?;
600+
// Check if this network uses SIGHASH_FORKID (BCH-style networks)
601+
let fork_id = match network.mainnet() {
602+
Network::BitcoinCash | Network::Ecash | Network::BitcoinSV => Some(0u32),
603+
Network::BitcoinGold => Some(79u32),
604+
_ => None,
605+
};
606+
607+
// Get input value for BIP143-style sighash (required for FORKID)
608+
let input = &psbt.inputs[input_index];
609+
let prevout = psbt.unsigned_tx.input[input_index].previous_output;
610+
let value = psbt_wallet_input::get_output_script_and_value(input, prevout)
611+
.map(|(_, v)| v)
612+
.unwrap_or(miniscript::bitcoin::Amount::ZERO);
613+
614+
// Compute sighash based on network type
615+
let mut cache = SighashCache::new(&psbt.unsigned_tx);
616+
let (message, sighash_type_u32) = if let Some(fork_id) = fork_id {
617+
// BCH-style BIP143 sighash with FORKID
618+
// SIGHASH_ALL | SIGHASH_FORKID = 0x01 | 0x40 = 0x41
619+
let sighash_type = 0x41u32;
620+
let sighash = cache
621+
.p2wsh_signature_hash_forkid(
622+
input_index,
623+
redeem_script,
624+
value,
625+
sighash_type,
626+
Some(fork_id),
627+
)
628+
.map_err(|e| format!("Failed to compute FORKID sighash: {}", e))?;
629+
(
630+
secp256k1::Message::from_digest(sighash.to_byte_array()),
631+
sighash_type,
632+
)
633+
} else {
634+
// Legacy P2SH sighash for standard Bitcoin
635+
let sighash_type = miniscript::bitcoin::sighash::EcdsaSighashType::All;
636+
let sighash = cache
637+
.legacy_signature_hash(input_index, redeem_script, sighash_type.to_u32())
638+
.map_err(|e| format!("Failed to compute sighash: {}", e))?;
639+
(
640+
secp256k1::Message::from_digest(sighash.to_byte_array()),
641+
sighash_type.to_u32(),
642+
)
643+
};
598644

599645
// Create ECDSA signature
600-
let message = secp256k1::Message::from_digest(sighash.to_byte_array());
601646
let signature = secp.sign_ecdsa(&message, privkey);
602647
let ecdsa_sig = EcdsaSignature {
603648
signature,
604-
sighash_type,
649+
sighash_type: sighash_type_u32,
605650
};
606651

607652
// Add signature to partial_sigs
@@ -688,7 +733,18 @@ impl BitGoPsbt {
688733
K: miniscript::bitcoin::psbt::GetKey,
689734
{
690735
match self {
691-
BitGoPsbt::BitcoinLike(ref mut psbt, _network) => psbt.sign(k, secp),
736+
BitGoPsbt::BitcoinLike(ref mut psbt, network) => {
737+
// Check if this network uses SIGHASH_FORKID
738+
// BCH, XEC, BSV: fork_id = 0
739+
// BTG: fork_id = 79
740+
match network.mainnet() {
741+
Network::BitcoinCash | Network::Ecash | Network::BitcoinSV => {
742+
psbt.sign_forkid(k, secp, 0)
743+
}
744+
Network::BitcoinGold => psbt.sign_forkid(k, secp, 79),
745+
_ => psbt.sign(k, secp),
746+
}
747+
}
692748
BitGoPsbt::Zcash(_zcash_psbt, _network) => {
693749
// Return an error indicating Zcash signing is not implemented
694750
Err((
@@ -911,6 +967,7 @@ impl BitGoPsbt {
911967
use miniscript::bitcoin::{hashes::Hash, sighash::SighashCache};
912968

913969
let psbt = self.psbt();
970+
let network = self.network();
914971

915972
// Check input index bounds
916973
if input_index >= psbt.inputs.len() {
@@ -920,10 +977,9 @@ impl BitGoPsbt {
920977
let input = &psbt.inputs[input_index];
921978
let prevout = psbt.unsigned_tx.input[input_index].previous_output;
922979

923-
// Get output script from input
924-
let (output_script, _value) =
925-
psbt_wallet_input::get_output_script_and_value(input, prevout)
926-
.map_err(|e| format!("Failed to get output script: {}", e))?;
980+
// Get output script and value from input
981+
let (output_script, value) = psbt_wallet_input::get_output_script_and_value(input, prevout)
982+
.map_err(|e| format!("Failed to get output script: {}", e))?;
927983

928984
// Verify this is a replay protection input
929985
if !replay_protection.is_replay_protection_input(output_script) {
@@ -951,14 +1007,37 @@ impl BitGoPsbt {
9511007
return Ok(false);
9521008
};
9531009

954-
// Compute legacy P2SH sighash
955-
let cache = SighashCache::new(&psbt.unsigned_tx);
956-
let sighash = cache
957-
.legacy_signature_hash(input_index, redeem_script, ecdsa_sig.sighash_type.to_u32())
958-
.map_err(|e| format!("Failed to compute sighash: {}", e))?;
1010+
// Check if this network uses SIGHASH_FORKID
1011+
let fork_id = match network.mainnet() {
1012+
Network::BitcoinCash | Network::Ecash | Network::BitcoinSV => Some(0u32),
1013+
Network::BitcoinGold => Some(79u32),
1014+
_ => None,
1015+
};
1016+
1017+
// Compute sighash based on network type
1018+
let mut cache = SighashCache::new(&psbt.unsigned_tx);
1019+
let message = if let Some(fork_id) = fork_id {
1020+
// BCH-style BIP143 sighash with FORKID
1021+
// Use p2wsh_signature_hash_forkid which handles the forkid encoding
1022+
let sighash = cache
1023+
.p2wsh_signature_hash_forkid(
1024+
input_index,
1025+
redeem_script,
1026+
value,
1027+
ecdsa_sig.sighash_type as u32,
1028+
Some(fork_id),
1029+
)
1030+
.map_err(|e| format!("Failed to compute FORKID sighash: {}", e))?;
1031+
secp256k1::Message::from_digest(sighash.to_byte_array())
1032+
} else {
1033+
// Legacy P2SH sighash for standard Bitcoin
1034+
let sighash = cache
1035+
.legacy_signature_hash(input_index, redeem_script, ecdsa_sig.sighash_type)
1036+
.map_err(|e| format!("Failed to compute sighash: {}", e))?;
1037+
secp256k1::Message::from_digest(sighash.to_byte_array())
1038+
};
9591039

960-
// Verify the signature using the bitcoin crate's built-in verification
961-
let message = secp256k1::Message::from_digest(sighash.to_byte_array());
1040+
// Verify the signature
9621041
match secp.verify_ecdsa(&message, &ecdsa_sig.signature, &public_key.inner) {
9631042
Ok(()) => Ok(true),
9641043
Err(_) => Ok(false),
@@ -986,6 +1065,7 @@ impl BitGoPsbt {
9861065
public_key: CompressedPublicKey,
9871066
) -> Result<bool, String> {
9881067
let psbt = self.psbt();
1068+
let network = self.network();
9891069

9901070
let input = &psbt.inputs[input_index];
9911071

@@ -999,8 +1079,15 @@ impl BitGoPsbt {
9991079
);
10001080
}
10011081

1082+
// Determine fork_id based on network
1083+
let fork_id = match network.mainnet() {
1084+
Network::BitcoinCash | Network::Ecash | Network::BitcoinSV => Some(0u32),
1085+
Network::BitcoinGold => Some(79u32),
1086+
_ => None,
1087+
};
1088+
10021089
// Fall back to ECDSA signature verification for legacy/SegWit inputs
1003-
psbt_wallet_input::verify_ecdsa_signature(secp, psbt, input_index, public_key)
1090+
psbt_wallet_input::verify_ecdsa_signature(secp, psbt, input_index, public_key, fork_id)
10041091
}
10051092

10061093
/// Verify if a valid signature exists for a given extended public key at the specified input index
@@ -1584,6 +1671,62 @@ mod tests {
15841671
Ok(())
15851672
}
15861673

1674+
/// Test that sign_with_privkey → verify_replay_protection_signature roundtrip works.
1675+
///
1676+
/// This test guards against sighash algorithm mismatches between signing and verification.
1677+
/// Specifically, it catches the bug where sign_with_privkey used legacy_signature_hash
1678+
/// for all networks, but verify_replay_protection_signature used p2wsh_signature_hash_forkid
1679+
/// for BCH-like networks (BitcoinCash, BitcoinGold, Ecash).
1680+
fn assert_p2shp2pk_sign_verify_roundtrip(
1681+
unsigned_fixture: &fixtures::PsbtFixture,
1682+
wallet_keys: &fixtures::XprvTriple,
1683+
input_index: usize,
1684+
network: Network,
1685+
) -> Result<(), String> {
1686+
// Get the xpriv for signing (user key)
1687+
let xpriv = wallet_keys.user_key();
1688+
let privkey = xpriv.private_key;
1689+
1690+
// Deserialize the unsigned PSBT
1691+
let original_bytes = BASE64_STANDARD
1692+
.decode(&unsigned_fixture.psbt_base64)
1693+
.map_err(|e| format!("Failed to decode base64: {}", e))?;
1694+
let mut psbt = BitGoPsbt::deserialize(&original_bytes, network)
1695+
.map_err(|e| format!("Failed to deserialize PSBT: {:?}", e))?;
1696+
1697+
// Sign the p2shP2pk input
1698+
psbt.sign_with_privkey(input_index, &privkey)
1699+
.map_err(|e| format!("Failed to sign p2shP2pk input: {}", e))?;
1700+
1701+
// Get the output script for replay protection verification
1702+
let psbt_ref = psbt.psbt();
1703+
let input = &psbt_ref.inputs[input_index];
1704+
let prevout = psbt_ref.unsigned_tx.input[input_index].previous_output;
1705+
let (output_script, _value) =
1706+
psbt_wallet_input::get_output_script_and_value(input, prevout)
1707+
.map_err(|e| format!("Failed to get output script: {}", e))?;
1708+
1709+
let replay_protection =
1710+
crate::fixed_script_wallet::ReplayProtection::new(vec![output_script.clone()]);
1711+
1712+
// Verify the signature
1713+
let secp = secp256k1::Secp256k1::new();
1714+
let has_valid_signature = psbt
1715+
.verify_replay_protection_signature(&secp, input_index, &replay_protection)
1716+
.map_err(|e| format!("Failed to verify signature: {}", e))?;
1717+
1718+
if !has_valid_signature {
1719+
return Err(format!(
1720+
"p2shP2pk sign→verify roundtrip failed for {:?}. \
1721+
This indicates a sighash mismatch between sign_with_privkey and \
1722+
verify_replay_protection_signature (e.g., SIGHASH_FORKID handling).",
1723+
network
1724+
));
1725+
}
1726+
1727+
Ok(())
1728+
}
1729+
15871730
fn assert_signature_count(
15881731
bitgo_psbt: &BitGoPsbt,
15891732
wallet_keys: &RootWalletKeys,
@@ -1685,6 +1828,17 @@ mod tests {
16851828
&psbt_input_stages.wallet_keys,
16861829
psbt_input_stages.input_index,
16871830
)?;
1831+
1832+
// Test sign→verify roundtrip from unsigned state.
1833+
// This verifies that sign_with_privkey uses the correct sighash algorithm:
1834+
// - BCH-like networks (BitcoinCash, BitcoinGold, Ecash): SIGHASH_FORKID | SIGHASH_ALL
1835+
// - Standard networks: SIGHASH_ALL (legacy)
1836+
assert_p2shp2pk_sign_verify_roundtrip(
1837+
&psbt_stages.unsigned,
1838+
&psbt_input_stages.wallet_keys,
1839+
psbt_input_stages.input_index,
1840+
network,
1841+
)?;
16881842
} else {
16891843
assert_full_signed_matches_wallet_scripts(
16901844
network,
@@ -1722,42 +1876,24 @@ mod tests {
17221876
crate::test_psbt_fixtures!(test_p2sh_p2pk_suite, network, format, {
17231877
test_wallet_script_type(fixtures::ScriptType::P2shP2pk, network, format).unwrap();
17241878
}, ignore: [
1725-
// TODO: sighash support
1726-
BitcoinCash, Ecash, BitcoinGold,
17271879
// TODO: zec support
17281880
Zcash,
17291881
]);
17301882

17311883
crate::test_psbt_fixtures!(test_p2sh_suite, network, format, {
17321884
test_wallet_script_type(fixtures::ScriptType::P2sh, network, format).unwrap();
17331885
}, ignore: [
1734-
// TODO: sighash support
1735-
BitcoinCash, Ecash, BitcoinGold,
17361886
// TODO: zec support
17371887
Zcash,
17381888
]);
17391889

1740-
crate::test_psbt_fixtures!(
1741-
test_p2sh_p2wsh_suite,
1742-
network,
1743-
format,
1744-
{
1745-
test_wallet_script_type(fixtures::ScriptType::P2shP2wsh, network, format).unwrap();
1746-
},
1747-
// TODO: sighash support
1748-
ignore: [BitcoinGold]
1749-
);
1890+
crate::test_psbt_fixtures!(test_p2sh_p2wsh_suite, network, format, {
1891+
test_wallet_script_type(fixtures::ScriptType::P2shP2wsh, network, format).unwrap();
1892+
});
17501893

1751-
crate::test_psbt_fixtures!(
1752-
test_p2wsh_suite,
1753-
network,
1754-
format,
1755-
{
1756-
test_wallet_script_type(fixtures::ScriptType::P2wsh, network, format).unwrap();
1757-
},
1758-
// TODO: sighash support
1759-
ignore: [BitcoinGold]
1760-
);
1894+
crate::test_psbt_fixtures!(test_p2wsh_suite, network, format, {
1895+
test_wallet_script_type(fixtures::ScriptType::P2wsh, network, format).unwrap();
1896+
});
17611897

17621898
crate::test_psbt_fixtures!(
17631899
test_p2tr_legacy_script_path_suite,
@@ -1822,7 +1958,7 @@ mod tests {
18221958
extracted_transaction_hex, fixture_extracted_transaction,
18231959
"Extracted transaction should match"
18241960
);
1825-
}, ignore: [BitcoinGold, BitcoinCash, Ecash, Zcash]);
1961+
}, ignore: [Zcash]);
18261962

18271963
#[test]
18281964
fn test_add_paygo_attestation() {
@@ -2116,7 +2252,7 @@ mod tests {
21162252
parsed.spend_amount > 0,
21172253
"Spend amount should be greater than 0 when there are external outputs"
21182254
);
2119-
}, ignore: [BitcoinGold, BitcoinCash, Ecash, Zcash]);
2255+
}, ignore: [Zcash]);
21202256

21212257
#[test]
21222258
fn test_serialize_bitcoin_psbt() {

0 commit comments

Comments
 (0)