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.

Sign up for GitHub

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

Jump to bottom

More features for strict mode #327

Merged
merged 4 commits into from
Oct 27, 2023
Merged

More features for strict mode #327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61ff9e2
fix: add more strict checks
friedger Oct 26, 2023
da51610
fix: retry on connection error
friedger Oct 26, 2023
41731d7
feat: ignore failed stx transactions
friedger Oct 26, 2023
13dc576
chore: extract constant
friedger Oct 27, 2023
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
5 changes: 5 additions & 0 deletions romeo/src/bitcoin_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@
))?;
}
}
Err(bitcoincore_rpc::Error::JsonRpc(
bitcoincore_rpc::jsonrpc::Error::Transport(_),
)) => {
trace!("Bitcoin client connection error, retrying...");

Check warning on line 175 in romeo/src/bitcoin_client.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/bitcoin_client.rs#L175 

Added line #L175 was not covered by tests
}
Err(err) => {
Err(anyhow!("Error fetching Bitcoin block: {:?}", err))?
}
Expand Down
79 changes: 60 additions & 19 deletions romeo/src/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
.into_iter()
.collect(),
Event::BitcoinTransactionUpdate(txid, status) => self
.process_bitcoin_transaction_update(txid, status)
.process_bitcoin_transaction_update(txid, status, config)
.into_iter()
.collect(),
Event::StacksBlock(height, txs) => {
Expand All @@ -157,15 +157,19 @@
.into_iter()
.collect(),
Event::MintBroadcasted(deposit_info, txid) => {
self.process_mint_broadcasted(deposit_info, txid);
self.process_mint_broadcasted(deposit_info, txid, config);
vec![]
}
Event::BurnBroadcasted(withdrawal_info, txid) => {
self.process_burn_broadcasted(withdrawal_info, txid);
self.process_burn_broadcasted(withdrawal_info, txid, config);
vec![]
}
Event::FulfillBroadcasted(withdrawal_info, txid) => {
self.process_fulfillment_broadcasted(withdrawal_info, txid);
self.process_fulfillment_broadcasted(
withdrawal_info,
txid,
config,
);
vec![]
}
}
Expand Down Expand Up @@ -310,17 +314,28 @@
has_pending_task,
} = req
else {
panic!("Got an {:?} status update for a Stacks transaction that is not acknowledged: {}", status, txid);
if config.strict {
panic!("Got an {:?} status update for a Stacks transaction that is not acknowledged: {}", status, txid);

Check warning on line 318 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L317-L318 

Added lines #L317 - L318 were not covered by tests
} else {
debug!("Ignoring {:?} status update for a Stacks transaction that is not acknowledged: {}", status, txid);
return false;

Check warning on line 321 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L320-L321 

Added lines #L320 - L321 were not covered by tests
}
};

if txid != *current_txid {
return false;
}

if !*has_pending_task {
panic!(
"Got an {:?} status update for a Stacks transaction that doesn't have a pending task: {}", status, txid
);
if config.strict {
panic!(
"Got an {:?} status update for a Stacks transaction that doesn't have a pending task: {}", status, txid
);

Check warning on line 333 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L330-L333 

Added lines #L330 - L333 were not covered by tests
} else {
debug!(
"Igno anring {:?} status update for a Stacks transaction that doesn't have a pending task: {}", status, txid
);

Check warning on line 337 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L335-L337 

Added lines #L335 - L337 were not covered by tests
}
}

*current_status = status.clone();
Expand Down Expand Up @@ -349,13 +364,18 @@
&mut self,
txid: BitcoinTxId,
status: TransactionStatus,
config: &Config,

Check warning on line 367 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L367 

Added line #L367 was not covered by tests
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
) -> impl IntoIterator<Item = Task> {
let State::Initialized { withdrawals, .. } = self else {
panic!("Cannot process Bitcoin transaction update when state is not initialized");
};

if status == TransactionStatus::Rejected {
panic!("Bitcoin transaction failed: {}", txid);
if config.strict {
panic!("Bitcoin transaction failed: {}", txid);

Check warning on line 375 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L374-L375 

Added lines #L374 - L375 were not covered by tests
} else {
debug!("Bitcoin transaction failed: {}", txid);

Check warning on line 377 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L377 

Added line #L377 was not covered by tests
}
}

let statuses_updated: usize = withdrawals
Expand All @@ -368,17 +388,28 @@
has_pending_task,
} = req
else {
panic!("Got an {:?} status update for a Bitcoin transaction that is not acknowledged: txid {} req {:?}", status, txid, req);
if config.strict {
panic!("Got an {:?} status update for a Bitcoin transaction that is not acknowledged: txid {} req {:?}", status, txid, req);

Check warning on line 392 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L391-L392 

Added lines #L391 - L392 were not covered by tests
} else {
debug!("Ignoring {:?} status update for a Bitcoin transaction that is not acknowledged: txid {} req {:?}", status, txid, req);
return false;

Check warning on line 395 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L394-L395 

Added lines #L394 - L395 were not covered by tests
};
};

if txid != *current_txid {
return false;
}

if !*has_pending_task {
if config.strict {

Check warning on line 404 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L404 

Added line #L404 was not covered by tests
panic!(
"Got an {:?} status update for a Bitcoin transaction that doesn't have a pending task: {}", status, txid
);
} else {
debug!(
"Ignoring {:?} status update for a Bitcoin transaction that doesn't have a pending task: {}", status, txid
);

Check warning on line 411 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L409-L411 

Added lines #L409 - L411 were not covered by tests
}
}

*current_status = status.clone();
Expand Down Expand Up @@ -630,6 +661,7 @@
&mut self,
deposit_info: DepositInfo,
txid: StacksTxId,
config: &Config,

Check warning on line 664 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L664 

Added line #L664 was not covered by tests
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
) {
let State::Initialized { deposits, .. } = self else {
panic!("Cannot process broadcasted mint if uninitialized")
Expand All @@ -640,10 +672,13 @@
.find(|deposit| deposit.info == deposit_info)
.expect("Could not find a deposit for the mint");

assert!(
matches!(deposit.mint, Some(TransactionRequest::Created)),
"Newly minted deposit already has mint acknowledged"
);
debug!("Mint broadcasted: {:?}", deposit.mint);
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
if config.strict {
assert!(
matches!(deposit.mint, Some(TransactionRequest::Created)),
"Newly minted deposit already has mint acknowledged"

Check warning on line 679 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L675-L679 

Added lines #L675 - L679 were not covered by tests
);
}

Check warning on line 681 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L681 

Added line #L681 was not covered by tests

deposit.mint = Some(TransactionRequest::Acknowledged {
txid,
Expand All @@ -656,6 +691,7 @@
&mut self,
withdrawal_info: WithdrawalInfo,
txid: StacksTxId,
config: &Config,

Check warning on line 694 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L694 

Added line #L694 was not covered by tests
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
) {
let State::Initialized { withdrawals, .. } = self else {
panic!("Cannot process broadcasted burn if uninitialized")
Expand All @@ -666,10 +702,12 @@
.find(|withdrawal| withdrawal.info == withdrawal_info)
.expect("Could not find a withdrawal for the burn");

assert!(
matches!(withdrawal.burn, Some(TransactionRequest::Created)),
"Newly burned withdrawal already has burn acknowledged"
);
if config.strict {
assert!(
matches!(withdrawal.burn, Some(TransactionRequest::Created)),
"Newly burned withdrawal already has burn acknowledged"

Check warning on line 708 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L705-L708 

Added lines #L705 - L708 were not covered by tests
);
}

Check warning on line 710 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L710 

Added line #L710 was not covered by tests

withdrawal.burn = Some(TransactionRequest::Acknowledged {
txid,
Expand All @@ -682,6 +720,7 @@
&mut self,
withdrawal_info: WithdrawalInfo,
txid: BitcoinTxId,
config: &Config,

Check warning on line 723 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L723 

Added line #L723 was not covered by tests
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
) {
let State::Initialized { withdrawals, .. } = self else {
panic!("Cannot process broadcasted fulfillment if uninitialized")
Expand All @@ -692,10 +731,12 @@
.find(|withdrawal| withdrawal.info == withdrawal_info)
.expect("Could not find a withdrawal for the fulfillment");

assert!(
if config.strict {
assert!(

Check warning on line 735 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L734-L735 

Added lines #L734 - L735 were not covered by tests
matches!(withdrawal.fulfillment, Some(TransactionRequest::Created)),
"Newly fulfilled withdrawal already has fulfillment acknowledged"
);
}

Check warning on line 739 in romeo/src/state.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/state.rs#L739 

Added line #L739 was not covered by tests

withdrawal.fulfillment = Some(TransactionRequest::Acknowledged {
txid,
Expand Down
53 changes: 35 additions & 18 deletions romeo/src/system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
sync::mpsc,
task::JoinHandle,
};
use tracing::{info, trace};
use tracing::{debug, info, trace};

use crate::{
bitcoin_client::Client as BitcoinClient,
Expand All @@ -34,6 +34,11 @@
task::Task,
};

const DUMMY_STACKS_ID: StacksTxId = StacksTxId([
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0,
]);

/// The main run loop of this system.
/// This function feeds all events to the `state::update` function and spawns
/// all tasks returned from this function.
Expand Down Expand Up @@ -252,7 +257,7 @@
.await
.sign_and_broadcast(tx)
.await
.expect("Unable to sign and broadcast the mint transaction");
.expect("Unable to sign and broadcast the set public key transaction");

Check warning on line 260 in romeo/src/system.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/system.rs#L260 

Added line #L260 was not covered by tests

Event::ContractPublicKeySetBroadcasted(txid)
}
Expand Down Expand Up @@ -309,14 +314,20 @@

let tx = StacksTransaction::new(tx_version, tx_auth, tx_payload);

let txid = stacks_client
.lock()
.await
.sign_and_broadcast(tx)
.await
.expect("Unable to sign and broadcast the mint transaction");

Event::MintBroadcasted(deposit_info, txid)
match stacks_client.lock().await.sign_and_broadcast(tx).await {
Ok(txid) => Event::MintBroadcasted(deposit_info, txid),
Err(err) => {
if config.strict {
panic!(
"Unable to sign and broadcast the mint transaction: {}",
err
);

Check warning on line 324 in romeo/src/system.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/system.rs#L317-L324 

Added lines #L317 - L324 were not covered by tests
} else {
debug!("Ignoring failure to sign and broadcast the mint transaction: {}", err);
Event::MintBroadcasted(deposit_info, DUMMY_STACKS_ID)

Check warning on line 327 in romeo/src/system.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/system.rs#L326-L327 

Added lines #L326 - L327 were not covered by tests
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

async fn burn_asset(
Expand Down Expand Up @@ -371,14 +382,20 @@

let tx = StacksTransaction::new(tx_version, tx_auth, tx_payload);

let txid = stacks_client
.lock()
.await
.sign_and_broadcast(tx)
.await
.expect("Unable to sign and broadcast the mint transaction");

Event::BurnBroadcasted(withdrawal_info, txid)
match stacks_client.lock().await.sign_and_broadcast(tx).await {
Ok(txid) => Event::BurnBroadcasted(withdrawal_info, txid),
Err(err) => {
if config.strict {
panic!(
"Unable to sign and broadcast the burn transaction: {}",
err
);

Check warning on line 392 in romeo/src/system.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/system.rs#L385-L392 

Added lines #L385 - L392 were not covered by tests
} else {
debug!("Ignoring failure to sign and broadcast the burn transaction: {}", err);
Event::BurnBroadcasted(withdrawal_info, DUMMY_STACKS_ID)

Check warning on line 395 in romeo/src/system.rs

View check run for this annotation

Codecov / codecov/patch

romeo/src/system.rs#L394-L395 

Added lines #L394 - L395 were not covered by tests
CAGS295 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

async fn fulfill_asset(
Expand Down
Loading