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

chore: more documentations and remove duplicated code #28

Merged
merged 1 commit into from
May 6, 2024
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
49 changes: 12 additions & 37 deletions src/cli/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use ckb_sdk::{
TransactionBuilderConfiguration,
},
types::{
Address as CkbAddress, AddressPayload as CkbAddressPayload, NetworkInfo, ScriptGroup,
TransactionWithScriptGroups,
Address as CkbAddress, AddressPayload as CkbAddressPayload, NetworkInfo, NetworkType,
ScriptGroup, TransactionWithScriptGroups,
},
SECP256K1,
};
Expand Down Expand Up @@ -75,9 +75,8 @@ pub struct Args {
#[arg(long, value_parser = value_parsers::OutPointValueParser)]
pub(crate) spv_contract_out_point: OutPoint,

/// The owner of Bitcoin SPV cells.
#[arg(long, value_parser = value_parsers::AddressValueParser)]
pub(crate) spv_owner: CkbAddress,
#[clap(flatten)]
pub(crate) spv_owner: super::SpvOwner,

/// Disable the on-chain difficulty check.
///
Expand Down Expand Up @@ -110,9 +109,14 @@ impl Args {
pub fn execute(&self) -> Result<()> {
log::info!("Try to initialize a Bitcoin SPV instance on CKB");

if self.disable_difficulty_check && self.ckb.network == NetworkType::Mainnet {
let msg = "For safety, the option `self.disable_difficulty_check` \
are not allowed on the mainnet";
return Err(Error::other(msg));
}

self.check_inputs()?;
log::info!("The bitcoin start height is {}", self.bitcoin_start_height);
self.check_remotes()?;

let btc_start_header = self
.bitcoin
Expand Down Expand Up @@ -228,7 +232,7 @@ impl Args {
Error::other(msg)
})?;
let spv_cell = CellOutput::new_builder()
.lock((&self.spv_owner).into())
.lock(self.spv_owner.lock_script())
.type_(Some(spv_type_script).pack())
.build();
let spv_info = spv_cell
Expand Down Expand Up @@ -389,36 +393,7 @@ impl Args {
}

fn check_inputs(&self) -> Result<()> {
if self.spv_owner.network() != self.ckb.network {
let msg = "The input addresses and the selected network are not matched";
return Err(Error::cli(msg));
}

if self.spv_clients_count < 3 {
let msg = format!(
"The Bitcoint SPV clients count should be 3 at least but got {}",
self.spv_clients_count
);
return Err(Error::cli(msg));
}

if self.bitcoin_start_height % DIFFCHANGE_INTERVAL != 0 {
let msg = format!(
"invalid Bitcoint start height, expected multiples of \
{DIFFCHANGE_INTERVAL} but got {}",
self.bitcoin_start_height
);
return Err(Error::cli(msg));
}

Ok(())
}

fn check_remotes(&self) -> Result<()> {
if self.spv_owner.network() != self.ckb.network {
let msg = "The input addresses and the selected network are not matched";
return Err(Error::cli(msg));
}
self.spv_owner.check_network(self.ckb.network)?;

if self.spv_clients_count < 3 {
let msg = format!(
Expand Down
110 changes: 104 additions & 6 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
//! The command line argument.

use ckb_sdk::{rpc::CkbRpcClient, types::NetworkType};
use ckb_types::core::FeeRate;
use ckb_sdk::{
rpc::CkbRpcClient,
types::{Address, NetworkType},
};
use ckb_types::{core::FeeRate, packed::Script};
use clap::{Args, Parser, Subcommand};
use clap_verbosity_flag::{InfoLevel, Verbosity};
use url::Url;

use crate::{
components::BitcoinClient,
prelude::*,
result::Result,
result::{Error, Result},
utilities::{value_parsers, Key256Bits},
};

Expand Down Expand Up @@ -96,6 +99,55 @@ pub struct CkbRoArgs {
pub(crate) network: NetworkType,
}

#[derive(Parser)]
pub struct SpvOwner {
/// The owner of Bitcoin SPV cells.
///
/// ### Warnings
///
/// The owner should be an address which uses a ACP-like script.
///
/// Current standard ACP lock isn't satisfied, because it's has the
/// following limits:
///
/// > if 2 input cells are using the same type script, or are both missing
/// type scripts, the lock returns with an error state.
///
/// > if 2 output cells are using the same type script, or are both missing
/// type scripts, the lock returns with an error state.
///
/// ### References
///
/// - [Anyone-Can-Pay Lock (a.k.a ACP)](https://github.com/nervosnetwork/rfcs/blob/198fc90ab7582953ed85a6655e88e51346857475/rfcs/0026-anyone-can-pay/0026-anyone-can-pay.md)
#[arg(long, value_parser = value_parsers::AddressValueParser)]
pub(crate) spv_owner: Address,
}

#[derive(Parser)]
pub struct SpvOwnerOpt {
/// The owner of Bitcoin SPV cells.
/// If no owner is provided, the previous owner will be kept.
///
/// ### Warnings
///
/// The owner should be an address which uses a ACP-like script.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested addition:

"Otherwise unintended results may occur. If the unlocking of the current spv instance requires a signature, update and reorg operations may produce irreversible failures when this SpvOwner fills in another address that requires a signature to unlock."

///
/// Current standard ACP lock isn't satisfied, because it's has the
/// following limits:
///
/// > if 2 input cells are using the same type script, or are both missing
/// type scripts, the lock returns with an error state.
///
/// > if 2 output cells are using the same type script, or are both missing
/// type scripts, the lock returns with an error state.
///
/// ### References
///
/// - [Anyone-Can-Pay Lock (a.k.a ACP)](https://github.com/nervosnetwork/rfcs/blob/198fc90ab7582953ed85a6655e88e51346857475/rfcs/0026-anyone-can-pay/0026-anyone-can-pay.md)
#[arg(long, value_parser = value_parsers::AddressValueParser)]
pub(crate) spv_owner: Option<Address>,
}

#[derive(Args)]
#[group(multiple = false)]
pub struct FeeRateArgs {
Expand All @@ -110,16 +162,20 @@ pub struct FeeRateArgs {

/// [Experimental] Enable dynamic fee rate for CKB transactions.
///
/// The actual fee rate will be the `median` fee rate which is fetched through the CKB RPC method `get_fee_rate_statistics`.
/// The actual fee rate will be the `median` fee rate which is fetched
/// through the CKB RPC method `get_fee_rate_statistics`.
///
/// For security, a hard limit is required.
/// When the returned dynamic fee rate is larger than the hard limit, the hard limit will be used.
/// When the returned dynamic fee rate is larger than the hard limit, the
/// hard limit will be used.
///
/// ### Warning
///
/// Users have to make sure the remote CKB node they used are trustsed.
///
/// Ref: <https://github.com/nervosnetwork/ckb/tree/v0.114.0/rpc#method-get_fee_rate_statistics>
/// ### References
///
/// - [CKB JSON-RPC method `get_fee_rate_statistics`](https://github.com/nervosnetwork/ckb/tree/v0.114.0/rpc#method-get_fee_rate_statistics)
#[arg(
group = "dynamic-fee-rate",
conflicts_with = "fixed-fee-rate",
Expand Down Expand Up @@ -224,6 +280,48 @@ impl CkbRoArgs {
}
}

impl AsRef<Address> for SpvOwner {
fn as_ref(&self) -> &Address {
&self.spv_owner
}
}

impl SpvOwner {
pub fn check_network(&self, expected: NetworkType) -> Result<()> {
let actual = self.as_ref().network();
if actual == expected {
Ok(())
} else {
let msg = "The input addresses and the selected network are not matched";
Err(Error::cli(msg))
}
}

pub fn lock_script(&self) -> Script {
self.as_ref().into()
}
}

impl SpvOwnerOpt {
pub fn check_network(&self, expected: NetworkType) -> Result<()> {
let is_same_network = self
.spv_owner
.as_ref()
.map(|actual| actual.network() == expected)
.unwrap_or(true);
if is_same_network {
Ok(())
} else {
let msg = "The input addresses and the selected network are not matched";
Err(Error::cli(msg))
}
}

pub fn lock_script(&self) -> Option<Script> {
self.spv_owner.as_ref().map(Into::into)
}
}

impl BitcoinArgs {
pub fn client(&self) -> BitcoinClient {
BitcoinClient::new(
Expand Down
28 changes: 12 additions & 16 deletions src/cli/serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::{
constants,
prelude::*,
result::{Error, Result},
utilities::{try_raise_fd_limit, value_parsers},
utilities::try_raise_fd_limit,
};

#[derive(Parser)]
Expand Down Expand Up @@ -80,9 +80,8 @@ pub struct Args {
#[arg(long, default_value = "30")]
pub(crate) bitcoin_headers_download_batch_size: u32,

/// The owner of Bitcoin SPV cells.
#[arg(long, value_parser = value_parsers::AddressValueParser)]
pub(crate) spv_owner: Option<CkbAddress>,
#[clap(flatten)]
pub(crate) spv_owner_opt: super::SpvOwnerOpt,

/// Perform all steps without sending.
#[arg(long, hide = true)]
Expand All @@ -93,12 +92,7 @@ impl Args {
pub fn execute(&self) -> Result<()> {
log::info!("Starting the Bitcoin SPV service");

if let Some(ref addr) = self.spv_owner {
if addr.network() != self.ckb.network {
let msg = "The input addresses and the selected network are not matched";
return Err(Error::cli(msg));
}
}
self.spv_owner_opt.check_network(self.ckb.network)?;

try_raise_fd_limit();

Expand Down Expand Up @@ -271,7 +265,9 @@ impl Args {
let packed_spv_client: packed::SpvClient = spv_client.pack();
vec![packed_spv_info.as_bytes(), packed_spv_client.as_bytes()]
};
let spv_outputs: Vec<CellOutput> = if let Some(ref addr) = self.spv_owner {
let spv_outputs: Vec<CellOutput> = if let Some(lock_script) =
self.spv_owner_opt.lock_script()
{
let spv_info_capacity = Capacity::bytes(spv_outputs_data[0].len()).map_err(|err| {
let msg = format!(
"failed to calculate the capacity for Bitcoin SPV info cell since {err}"
Expand All @@ -291,7 +287,7 @@ impl Args {
.output
.clone()
.as_builder()
.lock(addr.into())
.lock(lock_script.clone())
.build_exact_capacity(spv_info_capacity)
.map_err(|err| {
let msg = format!(
Expand All @@ -305,7 +301,7 @@ impl Args {
.output
.clone()
.as_builder()
.lock(addr.into())
.lock(lock_script)
.build_exact_capacity(spv_client_capacity)
.map_err(|err| {
let msg = format!(
Expand Down Expand Up @@ -539,7 +535,7 @@ impl Args {
}
outputs_data
};
let spv_outputs = if let Some(ref addr) = self.spv_owner {
let spv_outputs = if let Some(lock_script) = self.spv_owner_opt.lock_script() {
let spv_info_capacity = Capacity::bytes(spv_outputs_data[0].len()).map_err(|err| {
let msg = format!(
"failed to calculate the capacity for Bitcoin SPV info cell since {err}"
Expand All @@ -559,7 +555,7 @@ impl Args {
.output
.clone()
.as_builder()
.lock(addr.into())
.lock(lock_script.clone())
.build_exact_capacity(spv_info_capacity)
.map_err(|err| {
let msg = format!(
Expand All @@ -574,7 +570,7 @@ impl Args {
.output
.clone()
.as_builder()
.lock(addr.into())
.lock(lock_script.clone())
.build_exact_capacity(spv_client_capacity)
.map_err(|err| {
let msg = format!(
Expand Down