Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tr spend_info lock #4

Merged
merged 1 commit into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ mod tests {
use bitcoin::blockdata::opcodes::all::{OP_CLTV, OP_CSV};
use bitcoin::blockdata::script::Instruction;
use bitcoin::blockdata::{opcodes, script};
use bitcoin::hashes::hex::FromHex;
use bitcoin::hashes::hex::{FromHex, ToHex};
use bitcoin::hashes::{hash160, sha256};
use bitcoin::util::bip32;
use bitcoin::{self, secp256k1, EcdsaSigHashType, PublicKey};
Expand Down Expand Up @@ -1192,6 +1192,18 @@ mod tests {
)
}

#[test]
fn tr_script_pubkey() {
let key = Descriptor::<bitcoin::PublicKey>::from_str(
"tr(02e20e746af365e86647826397ba1c0e0d5cb685752976fe2f326ab76bdc4d6ee9)",
)
.unwrap();
assert_eq!(
key.script_pubkey().to_hex(),
"51209c19294f03757da3dc235a5960631e3c55751632f5889b06b7a053bdc0bcfbcb"
)
}

#[test]
fn roundtrip_tests() {
let descriptor = Descriptor::<bitcoin::PublicKey>::from_str("multi");
Expand Down
108 changes: 57 additions & 51 deletions src/descriptor/tr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,57 +212,63 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
{
// If the value is already cache, read it
// read only panics if the lock is poisoned (meaning other thread having a lock panicked)
if let Some(spend_info) = &*self.spend_info.read().expect("Lock poisoned") {
return Arc::clone(spend_info);
} else {
// Get a new secp context
// This would be cheap operation after static context support from upstream
let secp = secp256k1::Secp256k1::verification_only();
// Key spend path with no merkle root
let data = if self.tree.is_none() {
TaprootSpendInfo::new_key_spend(&secp, self.internal_key.to_x_only_pubkey(), None)
} else {
let mut builder = TaprootBuilder::new();
for (depth, ms) in self.iter_scripts() {
let script = ms.encode();
builder = builder
.add_leaf(depth, script)
.expect("Computing spend data on a valid Tree should always succeed");
}
// Assert builder cannot error here because we have a well formed descriptor
match builder.finalize(&secp, self.internal_key.to_x_only_pubkey()) {
Ok(data) => data,
Err(e) => match e {
TaprootBuilderError::InvalidMerkleTreeDepth(_) => {
unreachable!("Depth checked in struct construction")
}
TaprootBuilderError::NodeNotInDfsOrder => {
unreachable!("Insertion is called in DFS order")
}
TaprootBuilderError::OverCompleteTree => {
unreachable!("Taptree is a well formed tree")
}
TaprootBuilderError::InvalidInternalKey(_) => {
unreachable!("Internal key checked for validity")
}
TaprootBuilderError::IncompleteTree => {
unreachable!("Taptree is a well formed tree")
}
TaprootBuilderError::EmptyTree => {
unreachable!("Taptree is a well formed tree with atleast 1 element")
}
},
}
};
*self.spend_info.write().expect("Lock poisoned") = Some(Arc::new(data));
// Fetch the cached value
Arc::clone(
self.spend_info
.read()
.expect("Lock poisoned")
.as_ref() // deref from RwLockReadGuard to Option<TaprootSpendInfo>
.expect("Value cached above, option must be some"),
)
let spend_info = self
.spend_info
.read()
.expect("Lock poisoned")
.as_ref()
.map(Arc::clone);

match spend_info {
Some(spend_info) => spend_info,
None => {
// Get a new secp context
// This would be cheap operation after static context support from upstream
let secp = secp256k1::Secp256k1::verification_only();
// Key spend path with no merkle root
let data = if self.tree.is_none() {
TaprootSpendInfo::new_key_spend(
&secp,
self.internal_key.to_x_only_pubkey(),
None,
)
} else {
let mut builder = TaprootBuilder::new();
for (depth, ms) in self.iter_scripts() {
let script = ms.encode();
builder = builder
.add_leaf(depth, script)
.expect("Computing spend data on a valid Tree should always succeed");
}
// Assert builder cannot error here because we have a well formed descriptor
match builder.finalize(&secp, self.internal_key.to_x_only_pubkey()) {
Ok(data) => data,
Err(e) => match e {
TaprootBuilderError::InvalidMerkleTreeDepth(_) => {
unreachable!("Depth checked in struct construction")
}
TaprootBuilderError::NodeNotInDfsOrder => {
unreachable!("Insertion is called in DFS order")
}
TaprootBuilderError::OverCompleteTree => {
unreachable!("Taptree is a well formed tree")
}
TaprootBuilderError::InvalidInternalKey(_) => {
unreachable!("Internal key checked for validity")
}
TaprootBuilderError::IncompleteTree => {
unreachable!("Taptree is a well formed tree")
}
TaprootBuilderError::EmptyTree => {
unreachable!("Taptree is a well formed tree with atleast 1 element")
}
},
}
};
let spend_info = Arc::new(data);
*self.spend_info.write().expect("Lock poisoned") = Some(Arc::clone(&spend_info));
spend_info
}
}
}
}
Expand Down