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

340 add handling for non recoverable stellar submission errors #391

Merged
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
20763a5
Moved cache population from send_to_address to submit_transaction fun…
adelarja Aug 31, 2023
97504b8
Added draft implementation for error handling
adelarja Aug 31, 2023
90bc01c
removed pub visibility modifier from submit_transaction
adelarja Sep 1, 2023
df8bd51
Made struct fields public
adelarja Sep 1, 2023
5f17ed0
Changed submit_transaction function
adelarja Sep 7, 2023
3551576
Fixed unit test. Added mocktopus to Cargo.toml
adelarja Oct 3, 2023
a5de23e
Update Cargo.lock
ebma Oct 13, 2023
f9ee769
Refactor code
ebma Oct 13, 2023
a753b1d
Added draft implementation for error handling
adelarja Aug 31, 2023
8501970
re-add again Adel's code
b-yap Oct 25, 2023
a6dbcb7
cargo fmt
b-yap Oct 25, 2023
6e98910
incorporate Marcel's changes
b-yap Oct 27, 2023
825bc8c
cleanup after rebase
b-yap Oct 27, 2023
b326ef2
apply changes
b-yap Oct 31, 2023
ca0bd4e
update resubmission code
b-yap Nov 1, 2023
925363b
add test cases
b-yap Nov 1, 2023
1658281
update test cases and some functions
b-yap Nov 2, 2023
5b8f568
remove stray println
b-yap Nov 2, 2023
960e072
increase loop time interval
b-yap Nov 2, 2023
06c6703
increase loop time interval
b-yap Nov 2, 2023
6224c0d
remove the "break", as we want it to loop endlessly
b-yap Nov 2, 2023
1f37760
add logs and update the auto checker
b-yap Nov 2, 2023
e477354
cleanup logs
b-yap Nov 2, 2023
0fb9ea2
https://github.com/pendulum-chain/spacewalk/actions/runs/6732117257/j…
b-yap Nov 2, 2023
2b2ecd1
removed the idea of auto-check; immediately just delete from cache;
b-yap Nov 3, 2023
522911d
update unfinished comment
b-yap Nov 3, 2023
5088e24
update unfinished commenthttps://github.com/pendulum-chain/spacewalk/…
b-yap Nov 3, 2023
b6f4695
https://github.com/pendulum-chain/spacewalk/actions/runs/6743411899/j…
b-yap Nov 6, 2023
a1ee8c5
https://github.com/pendulum-chain/spacewalk/actions/runs/6768409960/j…
b-yap Nov 6, 2023
b07b4c8
https://github.com/pendulum-chain/spacewalk/pull/391#discussion_r1388…
b-yap Nov 10, 2023
31dc61f
https://github.com/pendulum-chain/spacewalk/actions/runs/6822042466/j…
b-yap Nov 13, 2023
bd0c2ce
https://github.com/pendulum-chain/spacewalk/actions/runs/6846493065/j…
b-yap Nov 13, 2023
beba015
cargo fmt
b-yap Nov 13, 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
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.

2 changes: 1 addition & 1 deletion clients/vault/src/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ pub async fn handle_replace_request<
&event.old_vault_id,
event.amount,
0, // do not lock any additional collateral
wallet.get_public_key_raw(),
wallet.public_key_raw(),
)
.await?)
}
Expand Down
12 changes: 5 additions & 7 deletions clients/vault/src/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use runtime::{
};
use service::{wait_or_shutdown, Error as ServiceError, MonitoringConfig, Service};
use stellar_relay_lib::{sdk::PublicKey, StellarOverlayConfig};
use wallet::{LedgerTxEnvMap, StellarWallet};
use wallet::{LedgerTxEnvMap, RESUBMISSION_INTERVAL_IN_SECS, StellarWallet};

use crate::{
cancellation::ReplaceCanceller,
Expand Down Expand Up @@ -704,7 +704,7 @@ impl VaultService {
let stellar_wallet = StellarWallet::from_secret_encoded(&secret_key, is_public_network)?;
tracing::debug!(
"Vault wallet public key: {}",
from_utf8(&stellar_wallet.get_public_key().to_encoding())?
from_utf8(&stellar_wallet.public_key().to_encoding())?
);

let stellar_wallet = Arc::new(RwLock::new(stellar_wallet));
Expand Down Expand Up @@ -759,13 +759,11 @@ impl VaultService {
self.vault_id_manager.fetch_vault_ids().await?;

let wallet = self.stellar_wallet.write().await;
let vault_public_key = wallet.get_public_key();
let vault_public_key = wallet.public_key();
let is_public_network = wallet.is_public_network();

// re-submit transactions in the cache
let _receivers = wallet.resubmit_transactions_from_cache().await;
//todo: handle errors from the receivers

wallet.start_periodic_resubmission_of_transactions_from_cache(RESUBMISSION_INTERVAL_IN_SECS).await;
drop(wallet);

let oracle_agent = self.create_oracle_agent(is_public_network).await?;
Expand Down Expand Up @@ -806,7 +804,7 @@ impl VaultService {
}

if self.spacewalk_parachain.get_public_key().await?.is_none() {
let public_key = self.stellar_wallet.read().await.get_public_key();
let public_key = self.stellar_wallet.read().await.public_key();
let pub_key_encoded = public_key.to_encoding();
tracing::info!(
"Registering public key to the parachain...{}",
Expand Down
2 changes: 1 addition & 1 deletion clients/vault/tests/helper/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub async fn register_vault_with_wallet(
items: Vec<(&SpacewalkParachain, &VaultId, u128)>,
) -> u128 {
let wallet_read = wallet.read().await;
let public_key = wallet_read.get_public_key();
let public_key = wallet_read.public_key();

let vault_collateral = register_vault(public_key, items).await;

Expand Down
40 changes: 20 additions & 20 deletions clients/vault/tests/vault_integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ async fn test_redeem_succeeds() {
),
async {
let wallet_read = user_wallet.read().await;
let address = wallet_read.get_public_key_raw();
let address = wallet_read.public_key_raw();
drop(wallet_read);
// We redeem half of what we issued
let redeem_id = user_provider
Expand Down Expand Up @@ -340,7 +340,7 @@ async fn test_cancel_scheduler_succeeds() {
let wallet_read = vault_wallet.read().await;
let issue_request_listener = vault::service::listen_for_issue_requests(
new_vault_provider.clone(),
wallet_read.get_public_key(),
wallet_read.public_key(),
issue_cancellation_event_tx.clone(),
issue_set.clone(),
memos_to_issue_ids.clone(),
Expand Down Expand Up @@ -473,7 +473,7 @@ async fn test_issue_cancel_succeeds() {
let memos_to_issue_ids = Arc::new(RwLock::new(IssueIdLookup::new()));

let issue_filter =
IssueFilter::new(&vault_wallet.read().await.get_public_key()).expect("Invalid filter");
IssueFilter::new(&vault_wallet.read().await.public_key()).expect("Invalid filter");

let issue_amount = upscaled_compatible_amount(100);
let vault_collateral = get_required_vault_collateral_for_issue(
Expand All @@ -489,7 +489,7 @@ async fn test_issue_cancel_succeeds() {
.register_vault_with_public_key(
&vault_id,
vault_collateral,
vault_wallet.read().await.get_public_key_raw(),
vault_wallet.read().await.public_key_raw(),
)
.await
);
Expand Down Expand Up @@ -524,7 +524,7 @@ async fn test_issue_cancel_succeeds() {
let wallet_read = vault_wallet.read().await;
let service = join3(
vault::service::listen_for_new_transactions(
wallet_read.get_public_key(),
wallet_read.public_key(),
wallet_read.is_public_network(),
slot_tx_env_map.clone(),
issue_set.clone(),
Expand All @@ -533,7 +533,7 @@ async fn test_issue_cancel_succeeds() {
),
vault::service::listen_for_issue_requests(
vault_provider.clone(),
wallet_read.get_public_key(),
wallet_read.public_key(),
issue_event_tx,
issue_set.clone(),
memos_to_issue_ids.clone(),
Expand Down Expand Up @@ -664,7 +664,7 @@ async fn test_automatic_issue_execution_succeeds() {
.register_vault_with_public_key(
&vault_id,
vault_collateral,
wallet_read.get_public_key_raw(),
wallet_read.public_key_raw(),
)
.await
);
Expand Down Expand Up @@ -710,16 +710,16 @@ async fn test_automatic_issue_execution_succeeds() {
};

let wallet_read = vault_wallet.read().await;
let issue_filter =
IssueFilter::new(&wallet_read.get_public_key()).expect("Invalid filter");
let issue_filter = IssueFilter::new(&wallet_read.public_key()).expect("Invalid filter");

let slot_tx_env_map = Arc::new(RwLock::new(HashMap::new()));

let issue_set = Arc::new(RwLock::new(IssueRequestsMap::new()));
let memos_to_issue_ids = Arc::new(RwLock::new(IssueIdLookup::new()));
let (issue_event_tx, _issue_event_rx) = mpsc::channel::<CancellationEvent>(16);
let service = join3(
vault::service::listen_for_new_transactions(
wallet_read.get_public_key(),
wallet_read.public_key(),
wallet_read.is_public_network(),
slot_tx_env_map.clone(),
issue_set.clone(),
Expand All @@ -728,7 +728,7 @@ async fn test_automatic_issue_execution_succeeds() {
),
vault::service::listen_for_issue_requests(
vault_provider.clone(),
wallet_read.get_public_key(),
wallet_read.public_key(),
issue_event_tx,
issue_set.clone(),
memos_to_issue_ids.clone(),
Expand Down Expand Up @@ -782,7 +782,7 @@ async fn test_automatic_issue_execution_succeeds_for_other_vault() {
.register_vault_with_public_key(
&vault1_id,
vault_collateral,
wallet_read.get_public_key_raw(),
wallet_read.public_key_raw(),
)
.await
);
Expand All @@ -791,7 +791,7 @@ async fn test_automatic_issue_execution_succeeds_for_other_vault() {
.register_vault_with_public_key(
&vault2_id,
vault_collateral,
wallet_read.get_public_key_raw(),
wallet_read.public_key_raw(),
)
.await
);
Expand Down Expand Up @@ -859,7 +859,7 @@ async fn test_automatic_issue_execution_succeeds_for_other_vault() {
};

let wallet_read = vault_wallet.read().await;
let vault_account_public_key = wallet_read.get_public_key();
let vault_account_public_key = wallet_read.public_key();
drop(wallet_read);
let issue_filter = IssueFilter::new(&vault_account_public_key).expect("Invalid filter");

Expand Down Expand Up @@ -927,7 +927,7 @@ async fn test_execute_open_requests_succeeds() {
.register_vault_with_public_key(
&vault_id,
vault_collateral,
wallet_read.get_public_key_raw(),
wallet_read.public_key_raw(),
)
.await
);
Expand All @@ -943,8 +943,8 @@ async fn test_execute_open_requests_succeeds() {
.await;

let wallet_read = user_wallet.read().await;
let address = wallet_read.get_public_key();
let address_raw = wallet_read.get_public_key_raw();
let address = wallet_read.public_key();
let address_raw = wallet_read.public_key_raw();
drop(wallet_read);
// Place redeem requests. 100_00000 is our minimum redeem amount with the current fee
// settings defined in the chain spec
Expand Down Expand Up @@ -1098,7 +1098,7 @@ async fn test_shutdown() {
.register_vault_with_public_key(
&sudo_vault_id,
vault_collateral,
vault_wallet.read().await.get_public_key_raw(),
vault_wallet.read().await.public_key_raw(),
)
.await
);
Expand Down Expand Up @@ -1146,13 +1146,13 @@ async fn test_requests_with_incompatible_amounts_fail() {
.await;

let wallet_read = vault_wallet.read().await;
let address = wallet_read.get_public_key_raw();
let address = wallet_read.public_key_raw();
assert_ok!(
vault_provider
.register_vault_with_public_key(
&vault_id,
vault_collateral,
wallet_read.get_public_key_raw()
wallet_read.public_key_raw()
)
.await
);
Expand Down
1 change: 1 addition & 0 deletions clients/wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ primitives = { package = "spacewalk-primitives", path = "../../primitives"}
[dev-dependencies]
mockall = "0.8.1"
serial_test = "0.9.0"
mocktopus = "0.8.0"
19 changes: 9 additions & 10 deletions clients/wallet/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,14 @@ impl WalletStateStorage {
}

/// Removes a transaction from the local folder
pub fn remove_tx_envelope(&self, sequence: SequenceNumber) -> Result<(), Error> {
pub fn remove_tx_envelope(&self, sequence: SequenceNumber) {
let full_file_path = format!("{}/{sequence}", self.txs_inner_dir());
remove_file(&full_file_path).map_err(|e| {
tracing::error!("Failed to delete file: {:?}", e);
Error::cache_error_with_seq(CacheErrorKind::DeleteFileFailed, sequence)
})
match remove_file(&full_file_path) {
Ok(_) => tracing::debug!("remove_tx_envelope(): Deleted file with sequence {sequence}"),
Err(e) => tracing::error!(
"remove_tx_envelope(): Failed to delete file with sequence {sequence}: {e:?}"
),
}
}

#[allow(dead_code)]
Expand Down Expand Up @@ -330,7 +332,7 @@ mod test {
extract_tx_envelope_from_path, parse_xdr_string_to_vec_u8, Error, WalletStateStorage,
},
error::CacheErrorKind,
test_helper::public_key_from_encoding,
mock::public_key_from_encoding,
};
use primitives::{
stellar::{
Expand Down Expand Up @@ -477,10 +479,7 @@ mod test {
let actual_tx = new_storage.get_tx_envelope(sequence).expect("a tx should be found");
assert_eq!(actual_tx, expected_tx);

assert!(new_storage.remove_tx_envelope(sequence).is_ok());

// removing a tx again will return an error.
assert_error(new_storage.remove_tx_envelope(sequence), CacheErrorKind::DeleteFileFailed);
new_storage.remove_tx_envelope(sequence);

// let's remove the entire directory
new_storage.remove_dir();
Expand Down
17 changes: 9 additions & 8 deletions clients/wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub enum Error {
title: String,
status: StatusCode,
reason: String,
result_code_op: Vec<String>,
envelope_xdr: Option<String>,
},
#[error("Could not parse string: {0}")]
Expand All @@ -29,6 +30,9 @@ pub enum Error {
#[error(transparent)]
CacheError(CacheError),

#[error("Transaction resubmission failed: {0}")]
ResubmissionError(String),

#[error("Cannot send payment to self")]
SelfPaymentError,
}
Expand All @@ -37,9 +41,7 @@ impl Error {
pub fn is_recoverable(&self) -> bool {
match self {
Error::HorizonResponseError(e) if e.is_timeout() => true,
Error::HorizonSubmissionError { title: _, status, reason: _, envelope_xdr: _ }
if *status == 504 =>
true,
Error::HorizonSubmissionError { status, .. } if *status == 504 => true,
Error::CacheError(e) => match e.kind {
CacheErrorKind::CreateDirectoryFailed |
CacheErrorKind::FileCreationFailed |
Expand All @@ -58,8 +60,7 @@ impl Error {
match self {
Error::HorizonResponseError(e) =>
e.status().map(|code| server_errors.contains(&code.as_u16())).unwrap_or(false),
Error::HorizonSubmissionError { title: _, status, reason: _, envelope_xdr: _ } =>
server_errors.contains(status),
Error::HorizonSubmissionError { status, .. } => server_errors.contains(status),
_ => false,
}
}
Expand Down Expand Up @@ -99,9 +100,9 @@ impl Error {
#[derive(Error, PartialEq, Eq)]
pub struct CacheError {
pub(crate) kind: CacheErrorKind,
path: Option<String>,
envelope: Option<TransactionEnvelope>,
sequence_number: Option<SequenceNumber>,
pub(crate) path: Option<String>,
pub(crate) envelope: Option<TransactionEnvelope>,
pub(crate) sequence_number: Option<SequenceNumber>,
}
impl Display for CacheError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
Expand Down
9 changes: 8 additions & 1 deletion clients/wallet/src/horizon/horizon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,13 @@ impl HorizonClient for reqwest::Client {
continue
},

Err(Error::HorizonSubmissionError { title, status, reason, envelope_xdr }) => {
Err(Error::HorizonSubmissionError {
title,
status,
reason,
result_code_op,
envelope_xdr,
}) => {
tracing::error!("submitting transaction with seq no: {seq_no:?}: failed with {title}, {reason}");
tracing::debug!("submitting transaction with seq no: {seq_no:?}: the envelope: {envelope_xdr:?}");
let envelope_xdr = envelope_xdr.or(Some(transaction_xdr.to_string()));
Expand All @@ -181,6 +187,7 @@ impl HorizonClient for reqwest::Client {
title,
status,
reason,
result_code_op,
envelope_xdr,
})
},
Expand Down
Loading
Loading