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

coordinator: get_spend_transaction #31

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ authors = ["Antoine Poinsot <[email protected]>"]
edition = "2018"

[dependencies]
bitcoinconsensus = "0.19.0-2"
revault_tx = { version = "0.5", features = ["use-serde"] }
revault_net = "0.3"

Expand Down
11 changes: 6 additions & 5 deletions doc/PLUGIN.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ An overview of the spending attempts at the current block.

##### Vault resource

| Field | Type | Description |
| ------------------ | ------- | -------------------------------------------- |
| `value` | integer | Value of the vault in satoshis |
| `deposit_outpoint` | string | Deposit outpoint of the vault |
| `unvault_tx` | string | Psbt of the unvault transaction of the vault |
| Field | Type | Description |
| ------------------ | -------------- | -------------------------------------------- |
| `value` | integer | Value of the vault in satoshis |
| `deposit_outpoint` | string | Deposit outpoint of the vault |
| `unvault_tx` | string | Psbt of the unvault transaction of the vault |
| `candidate_tx` | string or null | Hex encoded transaction spending the vault, null if the watchtower did not retrieve it from coordinator |
Copy link
Member Author

Choose a reason for hiding this comment

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

or base64 ?


#### Plugin Response

Expand Down
33 changes: 22 additions & 11 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ pub struct BitcoindConfig {
pub poll_interval_secs: Duration,
}

/// Everything we need to know for talking to coordinator
#[derive(Debug, Clone, Deserialize)]
pub struct CoordinatorConfig {
/// The Noise static public key of the sync server
#[serde(deserialize_with = "deserialize_noisepubkey")]
pub noise_key: NoisePubkey,
/// The host of the sync server (may be an IP or a hidden service)
/// TODO: change for enum handling multiple choice.
pub host: SocketAddr,
}

#[derive(Debug, Clone, Deserialize)]
pub struct ScriptsConfig {
#[serde(deserialize_with = "deserialize_fromstr")]
Expand All @@ -149,11 +160,8 @@ pub struct Config {
/// The Noise static public keys of "our" stakeholder
#[serde(deserialize_with = "deserialize_noisepubkey")]
pub stakeholder_noise_key: NoisePubkey,
/// The host of the sync server (may be an IP or a hidden service)
pub coordinator_host: String,
/// The Noise static public key of the sync server
#[serde(deserialize_with = "deserialize_noisepubkey")]
pub coordinator_noise_key: NoisePubkey,
/// Everything we need to know to talk to coordinator
pub coordinator_config: Option<CoordinatorConfig>,
/// An optional custom data directory
// TODO: have a default implem as in cosignerd
pub data_dir: Option<PathBuf>,
Expand Down Expand Up @@ -352,8 +360,9 @@ mod tests {

stakeholder_noise_key = "3de4539519b6baca35ad14cd5bac9a4e0875a851632112405bb0547e6fcf16f6"

coordinator_host = "127.0.0.1:1"
coordinator_noise_key = "d91563973102454a7830137e92d0548bc83b4ea2799f1df04622ca1307381402"
[coordinator_config]
host = "127.0.0.1:1"
noise_key = "d91563973102454a7830137e92d0548bc83b4ea2799f1df04622ca1307381402"

[scripts_config]
cpfp_descriptor = "wsh(thresh(1,pk(xpub6BaZSKgpaVvibu2k78QsqeDWXp92xLHZxiu1WoqLB9hKhsBf3miBUDX7PJLgSPvkj66ThVHTqdnbXpeu8crXFmDUd4HeM4s4miQS2xsv3Qb/*)))#cwycq5xu"
Expand Down Expand Up @@ -384,8 +393,9 @@ mod tests {

stakeholder_noise_key = "3de4539519b6baca35ad14cd5bac9a4e0875a851632112405bb0547e6fcf16f6"

coordinator_host = "127.0.0.1:1"
coordinator_noise_key = "d91563973102454a7830137e92d0548bc83b4ea2799f1df04622ca1307381402"
[coordinator_config]
host = "127.0.0.1:1"
noise_key = "d91563973102454a7830137e92d0548bc83b4ea2799f1df04622ca1307381402"

[[plugins]]
path = "src/config.rs"
Expand Down Expand Up @@ -416,8 +426,9 @@ mod tests {

stakeholder_noise_key = "3de4539519b6baca35ad14cd5bac9a4e0875a851632112405bb0547e6fcf16f6"

coordinator_host = "127.0.0.1:1"
coordinator_noise_key = "d91563973102454a7830137e92d0548bc83b4ea2799f1df04622ca1307381402"
[coordinator_config]
host = "127.0.0.1:1"
noise_key = "d91563973102454a7830137e92d0548bc83b4ea2799f1df04622ca1307381402"

[scripts_config]
cpfp_descriptor = "wsh(thresh(1,pk(xpub6BaZSKgpaVvibu2k78QsqeDWXp92xLHZxiu1WoqLB9hKhsBf3miBUDX7PJLgSPvkj66ThVHTqdnbXpeu8crXFmDUd4HeM4s4miQS2xsv3Qb/*)))#cwycq5xu"
Expand Down
83 changes: 83 additions & 0 deletions src/coordinator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
use std::net::SocketAddr;

use revault_net::{
message::coordinator::{GetSpendTx, SpendTx},
noise::{PublicKey, SecretKey},
transport::KKTransport,
};

use revault_tx::bitcoin::{OutPoint, Transaction};

const COORDINATOR_CLIENT_RETRIES: usize = 3;

pub struct CoordinatorClient {
host: SocketAddr,
our_noise_secret_key: SecretKey,
pub_key: PublicKey,
/// How many times the client will try again
/// to send a request to coordinator upon failure
retries: usize,
}

impl CoordinatorClient {
pub fn new(our_noise_secret_key: SecretKey, host: SocketAddr, pub_key: PublicKey) -> Self {
Self {
host,
our_noise_secret_key,
pub_key,
retries: COORDINATOR_CLIENT_RETRIES,
}
}

/// Wrapper to retry a request sent to coordinator upon IO failure
/// according to the configured number of retries.
fn retry<T, R: Fn() -> Result<T, revault_net::Error>>(
&self,
request: R,
) -> Result<T, revault_net::Error> {
let mut error: Option<revault_net::Error> = None;
for _ in 0..self.retries {
match request() {
Ok(res) => return Ok(res),
Err(e) => error = Some(e),
}
log::debug!(
"Error while communicating with coordinator: {}, retrying",
error.as_ref().expect("An error must have happened"),
);
std::thread::sleep(std::time::Duration::from_secs(3));
}
Err(error.expect("An error must have happened"))
}

fn send_req<T>(&self, req: &revault_net::message::Request) -> Result<T, revault_net::Error>
where
T: serde::de::DeserializeOwned,
{
log::debug!(
"Sending request to Coordinator: '{}'",
serde_json::to_string(req).unwrap(),
);
let mut transport =
KKTransport::connect(self.host, &self.our_noise_secret_key, &self.pub_key)?;
transport.send_req(&req)
}

/// Get Spend transaction spending the vault with the given deposit outpoint.
/// Beware that the spend transaction may be invalid and needs to be verified against
/// libbitcoinconsensus.
Comment on lines +66 to +68
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of checking the Spend transaction against libbitcoinconsensus?

Copy link
Member Author

Choose a reason for hiding this comment

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

I may have misunderstood libbitcoinconsensus usage, but it was to check that the sigs and the tx are valid and can pass the btc contensus and that the coordinator did not trick us with a fake spend that will never pass the mempool

Copy link
Member

Choose a reason for hiding this comment

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

libbitcoinconsensus defines part of the consensus (and even more so of the standard Bitcoin Core relay policy). It can only do context-less checks. For instance, it would not be able to detect if the inputs of the Spend don't exist.

But if they committed to an invalid Spend.. It's their problem? It will be unspendable through the manager path so no harm can be done (assuming cosigning servers aren't compromised, which Spend things always do). They will have to Cancel all the Unvaults.

pub fn get_spend_transaction(
&self,
deposit_outpoint: OutPoint,
) -> Result<Option<Transaction>, revault_net::Error> {
let resp: SpendTx = self.retry(|| {
let msg = GetSpendTx { deposit_outpoint };
self.send_req(&msg.into())
})?;
log::debug!(
"Got from Coordinator: '{}'",
serde_json::to_string(&resp).unwrap()
);
Ok(resp.spend_tx)
}
}
4 changes: 3 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod bitcoind;
mod config;
mod coordinator;
mod daemonize;
mod database;
mod keys;
Expand Down Expand Up @@ -235,6 +236,7 @@ fn main() {
let _db_path = db_path.clone();
let _config = config.clone();
let _bitcoind = bitcoind.clone();
let noise_secret = noise_secret.clone();
move || {
listener_main(&_db_path, &_config, _bitcoind, &noise_secret).unwrap_or_else(|e| {
log::error!("Error in listener loop: '{}'", e);
Expand All @@ -246,7 +248,7 @@ fn main() {
log::info!("Started miradord.",);

let secp_ctx = secp256k1::Secp256k1::verification_only();
poller::main_loop(&db_path, &secp_ctx, &config, &bitcoind).unwrap_or_else(|e| {
poller::main_loop(&db_path, &secp_ctx, &config, &bitcoind, noise_secret).unwrap_or_else(|e| {
log::error!("Error in main loop: '{}'", e);
process::exit(1);
});
Expand Down
14 changes: 12 additions & 2 deletions src/plugins.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use revault_tx::{
bitcoin::{Amount, OutPoint},
bitcoin::{consensus::encode, Amount, OutPoint, Transaction},
transactions::UnvaultTransaction,
};

Expand Down Expand Up @@ -63,6 +63,12 @@ fn serialize_amount<S: Serializer>(amount: &Amount, serializer: S) -> Result<S::
amount.as_sat().serialize(serializer)
}

fn serialize_tx<S: Serializer>(tx: &Option<Transaction>, serializer: S) -> Result<S::Ok, S::Error> {
tx.as_ref()
.map(|tx| encode::serialize_hex(tx))
.serialize(serializer)
}

#[derive(Debug, Clone, Eq, PartialEq)]
pub struct Plugin {
path: path::PathBuf,
Expand All @@ -76,7 +82,8 @@ pub struct VaultInfo {
pub value: Amount,
pub deposit_outpoint: OutPoint,
pub unvault_tx: UnvaultTransaction,
// TODO: Spend tx
#[serde(serialize_with = "serialize_tx")]
pub candidate_tx: Option<Transaction>,
}

/// Information we are passing to a plugin after a new block if there was any update.
Expand Down Expand Up @@ -204,6 +211,7 @@ mod tests {
value: Amount::from_sat(567890),
deposit_outpoint,
unvault_tx: unvault_tx.clone(),
candidate_tx: None,
})
.collect();
let many_outpoints: Vec<OutPoint> = (0..10000).map(|_| deposit_outpoint).collect();
Expand Down Expand Up @@ -239,6 +247,7 @@ mod tests {
)
.unwrap(),
unvault_tx: unvault_tx.clone(),
candidate_tx: None,
};
let new_block = NewBlockInfo {
new_attempts: vec![vault_info],
Expand All @@ -263,6 +272,7 @@ mod tests {
value: Amount::from_sat(567890),
deposit_outpoint,
unvault_tx: unvault_tx.clone(),
candidate_tx: None,
};
let new_block = NewBlockInfo {
new_attempts: vec![vault_info],
Expand Down
Loading