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

Replacing unwraps with Errors in application code #492

Merged
merged 9 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 2 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
allow-unwrap-in-tests = true
allow-expect-in-tests = true
7 changes: 5 additions & 2 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ clap = { workspace = true, features = ["derive"] }
toml = { workspace = true }
sqlx = { workspace = true, features = ["runtime-tokio", "postgres", "macros"] }
header-chain = { workspace = true }
borsh = { workspace = true}
tonic = { workspace = true}
borsh = { workspace = true }
tonic = { workspace = true }
prost = { workspace = true }
tokio-stream = { workspace = true }
async-stream = { workspace = true }
Expand All @@ -48,3 +48,6 @@ testing = []
[[bin]]
name = "server"
path = "src/bin/server.rs"

[lints.clippy]
unwrap_used = { level = "deny", allow-unwrap-in-tests = true }
15 changes: 12 additions & 3 deletions core/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,13 @@ fn compile_protobuf() {

let proto_files: Vec<String> = protos
.iter()
.map(|proto| proto_root.join(proto).to_str().unwrap().to_owned())
.map(|proto| {
proto_root
.join(proto)
.to_str()
.expect("proto_root is not a valid path")
.to_owned()
})
.collect();

// Tell Cargo that if a proto file changes, rerun this build script.
Expand All @@ -45,8 +51,11 @@ fn compile_protobuf() {
.build_server(true)
.build_client(true)
.out_dir("./src/rpc")
.compile_protos(&proto_files, &[proto_root.to_str().unwrap()])
.unwrap();
.compile_protos(
&proto_files,
&[proto_root.to_str().expect("proto_root is not a valid path")],
)
.expect("Failed to compile protos");
}

fn main() {
Expand Down
2 changes: 2 additions & 0 deletions core/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
allow-unwrap-in-tests = true
allow-expect-in-tests = true
4 changes: 2 additions & 2 deletions core/src/aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl Aggregator {
self.config.user_takes_after,
self.config.bridge_amount_sats,
self.config.network,
);
)?;
// println!("MOVE_TX: {:?}", tx);
// println!("MOVE_TXID: {:?}", tx.tx.compute_txid());
let message = Message::from_digest(
Expand Down Expand Up @@ -354,7 +354,7 @@ impl Aggregator {
self.config.user_takes_after,
self.config.bridge_amount_sats,
self.config.network,
);
)?;
let move_tx_witness_elements = vec![move_tx_sig.serialize().to_vec()];
set_p2tr_script_spend_witness(&mut move_tx_handler, &move_tx_witness_elements, 0, 0)?;

Expand Down
28 changes: 20 additions & 8 deletions core/src/builder/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! addresses.

use crate::builder;
use crate::errors::BridgeError;
use crate::utils::SECP;
use crate::{utils, EVMAddress};
use bitcoin::address::NetworkUnchecked;
Expand All @@ -23,7 +24,11 @@ pub fn taproot_builder_with_scripts(scripts: &[ScriptBuf]) -> TaprootBuilder {
// Special return cases for n = 0 or n = 1
match num_scripts {
0 => return builder,
1 => return builder.add_leaf(0, scripts[0].clone()).unwrap(),
1 => {
return builder
.add_leaf(0, scripts[0].clone())
.expect("one root leaf added on empty builder")
}
_ => {}
}

Expand All @@ -39,7 +44,7 @@ pub fn taproot_builder_with_scripts(scripts: &[ScriptBuf]) -> TaprootBuilder {
deepest_layer_depth - is_node_in_last_minus_one_depth,
scripts[i].clone(),
)
.unwrap()
.expect("algorithm tested to be correct")
})
}

Expand Down Expand Up @@ -71,10 +76,12 @@ pub fn create_taproot_address(
let taproot_builder = taproot_builder_with_scripts(scripts);
// Finalize the tree
let tree_info = match internal_key {
Some(xonly_pk) => taproot_builder.finalize(&SECP, xonly_pk).unwrap(),
Some(xonly_pk) => taproot_builder
.finalize(&SECP, xonly_pk)
.expect("builder return is finalizable"),
None => taproot_builder
.finalize(&SECP, *utils::UNSPENDABLE_XONLY_PUBKEY)
.unwrap(),
.expect("builder return is finalizable"),
};
// Create the address
let taproot_address = match internal_key {
Expand Down Expand Up @@ -119,7 +126,7 @@ pub fn generate_deposit_address(
amount: Amount,
network: bitcoin::Network,
user_takes_after: u16,
) -> (Address, TaprootSpendInfo) {
) -> Result<(Address, TaprootSpendInfo), BridgeError> {
let deposit_script =
builder::script::create_deposit_script(nofn_xonly_pk, user_evm_address, amount);

Expand All @@ -128,14 +135,18 @@ pub fn generate_deposit_address(
.assume_checked()
.script_pubkey();
let recovery_extracted_xonly_pk =
XOnlyPublicKey::from_slice(&recovery_script_pubkey.as_bytes()[2..34]).unwrap();
XOnlyPublicKey::from_slice(&recovery_script_pubkey.as_bytes()[2..34])?;

let script_timelock = builder::script::generate_checksig_relative_timelock_script(
recovery_extracted_xonly_pk,
user_takes_after,
);

create_taproot_address(&[deposit_script, script_timelock], None, network)
Ok(create_taproot_address(
&[deposit_script, script_timelock],
None,
network,
))
}

/// Shorthand function for creating a checksig taproot address: A single checksig script with the given xonly PK and no internal key.
Expand Down Expand Up @@ -287,7 +298,8 @@ mod tests {
Amount::from_sat(100_000_000),
bitcoin::Network::Regtest,
200,
);
)
.unwrap();

// Comparing it to the taproot address generated in bridge backend.
assert_eq!(
Expand Down
4 changes: 2 additions & 2 deletions core/src/builder/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub fn anyone_can_spend_txout() -> TxOut {
pub fn anchor_output() -> TxOut {
TxOut {
value: ANCHOR_AMOUNT,
script_pubkey: ScriptBuf::from_hex("51024e73").unwrap(),
script_pubkey: ScriptBuf::from_hex("51024e73").expect("anchor script is valid"),
}
}

Expand All @@ -53,7 +53,7 @@ pub fn create_deposit_script(
evm_address: EVMAddress,
amount: Amount,
) -> ScriptBuf {
let citrea: [u8; 6] = "citrea".as_bytes().try_into().unwrap();
let citrea = b"citrea";

Builder::new()
.push_x_only_key(&nofn_xonly_pk)
Expand Down
4 changes: 2 additions & 2 deletions core/src/builder/sighash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub fn create_nofn_sighash_stream(
_user_takes_after,
bridge_amount_sats,
network,
);
)?;
// Get operator details (for each operator, (X-Only Public Key, Address, Collateral Funding Txid))
let operators: Vec<(XOnlyPublicKey, bitcoin::Address, Txid)> =
db.get_operators(None).await?;
Expand Down Expand Up @@ -349,7 +349,7 @@ pub fn create_operator_sighash_stream(
_user_takes_after,
bridge_amount_sats,
network,
);
)?;

let mut input_txid = collateral_funding_txid;
let mut input_amount = collateral_funding_amount;
Expand Down
9 changes: 5 additions & 4 deletions core/src/builder/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! transactions.

use crate::builder;
use crate::errors::BridgeError;
use crate::EVMAddress;
use bitcoin::address::NetworkUnchecked;
use bitcoin::Sequence;
Expand Down Expand Up @@ -119,7 +120,7 @@ pub fn create_move_to_vault_txhandler(
user_takes_after: u16,
bridge_amount_sats: Amount,
network: bitcoin::Network,
) -> TxHandler {
) -> Result<TxHandler, BridgeError> {
let (musig2_address, musig2_spendinfo) =
builder::address::create_checksig_address(nofn_xonly_pk, network);

Expand All @@ -140,7 +141,7 @@ pub fn create_move_to_vault_txhandler(
bridge_amount_sats,
network,
user_takes_after,
);
)?;

let prevouts = vec![TxOut {
script_pubkey: deposit_address.script_pubkey(),
Expand All @@ -153,15 +154,15 @@ pub fn create_move_to_vault_txhandler(
bridge_amount_sats,
)];

TxHandler {
Ok(TxHandler {
txid: move_tx.compute_txid(),
tx: move_tx,
prevouts,
prev_scripts: vec![deposit_script],
prev_taproot_spend_infos: vec![Some(deposit_taproot_spend_info)],
out_scripts: vec![vec![], vec![]],
out_taproot_spend_infos: vec![Some(musig2_spendinfo), None],
}
})
}

#[cfg(test)]
Expand Down
6 changes: 3 additions & 3 deletions core/src/builder/transaction/operator_assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,10 @@ pub fn create_assert_end_txhandler(
});

let disprove_taproot_spend_info = TaprootBuilder::new()
.add_hidden_node(0, TapNodeHash::from_slice(root_hash).unwrap())
.unwrap()
.add_hidden_node(0, TapNodeHash::from_byte_array(*root_hash))
.expect("empty taptree will accept a node at depth 0")
.finalize(&SECP, nofn_xonly_pk) // TODO: we should convert this to script spend but we only have partial access to the taptree
.unwrap();
.expect("finalize always succeeds for taptree with single node at depth 0");

let disprove_address = Address::p2tr(
&SECP,
Expand Down
Loading
Loading