Skip to content

Commit 840dcec

Browse files
authored
feat(sdk)!: retry broadcast operations (#2337)
1 parent 7393162 commit 840dcec

File tree

21 files changed

+333
-192
lines changed

21 files changed

+333
-192
lines changed

packages/dapi-grpc/build.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ impl MappingConfig {
289289
create_dir_all(&self.out_dir)?;
290290

291291
self.builder
292-
.compile(&[self.protobuf_file], &self.proto_includes)
292+
.compile_protos(&[self.protobuf_file], &self.proto_includes)
293293
}
294294
}
295295

packages/rs-dapi-client/src/dapi_client.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,11 @@ impl DapiRequestExecutor for DapiClient {
227227
.address_list
228228
.write()
229229
.expect("can't get address list for write");
230-
230+
tracing::warn!(
231+
?address,
232+
?error,
233+
"received server error, banning address"
234+
);
231235
address_list.ban_address(&address).map_err(|error| {
232236
ExecutionError {
233237
inner: DapiClientError::AddressList(error),
@@ -236,9 +240,18 @@ impl DapiRequestExecutor for DapiClient {
236240
address: Some(address.clone()),
237241
}
238242
})?;
243+
} else {
244+
tracing::debug!(
245+
?address,
246+
?error,
247+
"received server error, we should ban the node but banning is disabled"
248+
);
239249
}
240250
} else {
241-
tracing::trace!(?error, "received error");
251+
tracing::debug!(
252+
?error,
253+
"received server error, most likely the request is invalid"
254+
);
242255
}
243256
}
244257
};

packages/rs-dapi-client/src/executor.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,18 @@ where
124124
/// Result of request execution
125125
pub type ExecutionResult<R, E> = Result<ExecutionResponse<R>, ExecutionError<E>>;
126126

127+
impl<R, E> From<ExecutionResponse<R>> for ExecutionResult<R, E> {
128+
fn from(response: ExecutionResponse<R>) -> Self {
129+
ExecutionResult::<R, E>::Ok(response)
130+
}
131+
}
132+
133+
impl<R, E> From<ExecutionError<E>> for ExecutionResult<R, E> {
134+
fn from(e: ExecutionError<E>) -> Self {
135+
ExecutionResult::<R, E>::Err(e)
136+
}
137+
}
138+
127139
impl<R, E> IntoInner<Result<R, E>> for ExecutionResult<R, E> {
128140
fn into_inner(self) -> Result<R, E> {
129141
match self {
@@ -145,3 +157,64 @@ where
145157
}
146158
}
147159
}
160+
161+
/// Convert Result<T,TE> to ExecutionResult<R,E>, taking context from ExecutionResponse.
162+
pub trait WrapToExecutionResult<R, RE, W>: Sized {
163+
/// Convert self (eg. some [Result]) to [ExecutionResult], taking context information from `W` (eg. ExecutionResponse).
164+
///
165+
/// This function simplifies processing of results by wrapping them into ExecutionResult.
166+
/// It is useful when you have execution result retrieved in previous step and you want to
167+
/// add it to the result of the current step.
168+
///
169+
/// Useful when chaining multiple commands and you want to keep track of retries and address.
170+
///
171+
/// ## Example
172+
///
173+
/// ```rust
174+
/// use rs_dapi_client::{ExecutionResponse, ExecutionResult, WrapToExecutionResult};
175+
///
176+
/// fn some_request() -> ExecutionResult<i8, String> {
177+
/// Ok(ExecutionResponse {
178+
/// inner: 42,
179+
/// retries: 123,
180+
/// address: "http://127.0.0.1".parse().expect("create mock address"),
181+
/// })
182+
/// }
183+
///
184+
/// fn next_step() -> Result<i32, String> {
185+
/// Err("next error".to_string())
186+
/// }
187+
///
188+
/// let response = some_request().expect("request should succeed");
189+
/// let result: ExecutionResult<i32, String> = next_step().wrap_to_execution_result(&response);
190+
///
191+
/// if let ExecutionResult::Err(error) = result {
192+
/// assert_eq!(error.inner, "next error");
193+
/// assert_eq!(error.retries, 123);
194+
/// } else {
195+
/// panic!("Expected error");
196+
/// }
197+
/// ```
198+
fn wrap_to_execution_result(self, result: &W) -> ExecutionResult<R, RE>;
199+
}
200+
201+
impl<R, RE, TR, IR, IRE> WrapToExecutionResult<R, RE, ExecutionResponse<TR>> for Result<IR, IRE>
202+
where
203+
R: From<IR>,
204+
RE: From<IRE>,
205+
{
206+
fn wrap_to_execution_result(self, result: &ExecutionResponse<TR>) -> ExecutionResult<R, RE> {
207+
match self {
208+
Ok(r) => ExecutionResult::Ok(ExecutionResponse {
209+
inner: r.into(),
210+
retries: result.retries,
211+
address: result.address.clone(),
212+
}),
213+
Err(e) => ExecutionResult::Err(ExecutionError {
214+
inner: e.into(),
215+
retries: result.retries,
216+
address: Some(result.address.clone()),
217+
}),
218+
}
219+
}
220+
}

packages/rs-dapi-client/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ pub use dapi_client::{DapiClient, DapiClientError};
2222
pub use dump::DumpData;
2323
pub use executor::{
2424
DapiRequestExecutor, ExecutionError, ExecutionResponse, ExecutionResult, InnerInto, IntoInner,
25+
WrapToExecutionResult,
2526
};
2627
use futures::{future::BoxFuture, FutureExt};
2728
pub use request_settings::RequestSettings;

packages/rs-dapi-client/src/request_settings.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ const DEFAULT_BAN_FAILED_ADDRESS: bool = true;
1919
pub struct RequestSettings {
2020
/// Timeout for establishing a connection.
2121
pub connect_timeout: Option<Duration>,
22-
/// Timeout for a request.
22+
/// Timeout for single request (soft limit).
23+
///
24+
/// Note that the total maximum time of execution can exceed `(timeout + connect_timeout) * retries`
25+
/// as it accounts for internal processing time between retries.
2326
pub timeout: Option<Duration>,
2427
/// Number of retries in case of failed requests. If max retries reached, the last error is returned.
2528
/// 1 means one request and one retry in case of error, etc.

packages/rs-dapi-client/src/transport/grpc.rs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,38 @@ impl CanRetry for dapi_grpc::tonic::Status {
132132
}
133133
}
134134

135-
/// A shortcut to link between gRPC request type, response type, client and its
136-
/// method in order to represent it in a form of types and data.
135+
/// Macro to implement the `TransportRequest` trait for a given request type, response type, client type, and settings.
136+
///
137+
/// # Parameters
138+
///
139+
/// - `$request:ty`: The request type for which the `TransportRequest` trait will be implemented.
140+
/// - `$response:ty`: The response type returned by the transport request.
141+
/// - `$client:ty`: The client type used to execute the transport request (eg. generated by `tonic` crate).
142+
/// - `$settings:expr`: The settings to be used for the transport request; these settings will override client's
143+
/// default settings, but can still be overriden by arguments to
144+
/// the [`DapiRequestExecutor::execute`](crate::DapiRequestExecutor::execute) method.
145+
/// - `$($method:tt)+`: The method of `$client` to be called to execute the request.
146+
///
147+
/// # Example
148+
///
149+
/// ```compile_fail
150+
/// impl_transport_request_grpc!(
151+
/// MyRequestType,
152+
/// MyResponseType,
153+
/// MyClientType,
154+
/// my_settings,
155+
/// my_method
156+
/// );
157+
/// ```
158+
///
159+
/// This will generate an implementation of the `TransportRequest` trait for `MyRequestType`
160+
/// that uses `MyClientType` to execute the `my_method` method, with the specified `my_settings`.
161+
///
162+
/// The generated implementation will:
163+
/// - Define the associated types `Client` and `Response`.
164+
/// - Set the `SETTINGS_OVERRIDES` constant to the provided settings.
165+
/// - Implement the `method_name` function to return the name of the method as a string.
166+
/// - Implement the `execute_transport` function to execute the transport request using the provided client and settings.
137167
macro_rules! impl_transport_request_grpc {
138168
($request:ty, $response:ty, $client:ty, $settings:expr, $($method:tt)+) => {
139169
impl TransportRequest for $request {

packages/rs-dpp/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ dashcore = { git = "https://github.com/dashpay/rust-dashcore", features = [
2929
"signer",
3030
"serde",
3131
"bls",
32-
"eddsa"
32+
"eddsa",
3333
], default-features = false, tag = "0.32.0" }
3434
env_logger = { version = "0.11" }
3535
getrandom = { version = "0.2", features = ["js"] }
@@ -56,7 +56,7 @@ platform-version = { path = "../rs-platform-version" }
5656
platform-versioning = { path = "../rs-platform-versioning" }
5757
platform-serialization = { path = "../rs-platform-serialization" }
5858
platform-serialization-derive = { path = "../rs-platform-serialization-derive" }
59-
derive_more = { version = "1.0", features = ["from", "display"] }
59+
derive_more = { version = "1.0", features = ["from", "display", "try_into"] }
6060
nohash-hasher = "0.2.0"
6161
rust_decimal = "1.29.1"
6262
rust_decimal_macros = "1.29.1"

packages/rs-dpp/src/state_transition/proof_result.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::voting::votes::Vote;
55
use platform_value::Identifier;
66
use std::collections::BTreeMap;
77

8-
#[derive(Debug)]
8+
#[derive(Debug, strum::Display, derive_more::TryInto)]
99
pub enum StateTransitionProofResult {
1010
VerifiedDataContract(DataContract),
1111
VerifiedIdentity(Identity),

packages/rs-sdk/src/core/transaction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ impl Sdk {
5757
self.execute(core_transactions_stream, RequestSettings::default())
5858
.await
5959
.into_inner()
60-
.map_err(|e| Error::DapiClientError(e.to_string()))
60+
.map_err(|e| e.into())
6161
}
6262

6363
/// Waits for a response for the asset lock proof

packages/rs-sdk/src/error.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//! Definitions of errors
2+
use dapi_grpc::tonic::Code;
23
use dpp::consensus::ConsensusError;
34
use dpp::serialization::PlatformDeserializable;
45
use dpp::version::PlatformVersionError;
@@ -56,6 +57,10 @@ pub enum Error {
5657
/// SDK operation timeout reached error
5758
#[error("SDK operation timeout {} secs reached: {1}", .0.as_secs())]
5859
TimeoutReached(Duration, String),
60+
61+
/// Returned when an attempt is made to create an object that already exists in the system
62+
#[error("Object already exists: {0}")]
63+
AlreadyExists(String),
5964
/// Generic error
6065
// TODO: Use domain specific errors instead of generic ones
6166
#[error("SDK error: {0}")]
@@ -78,6 +83,7 @@ pub enum Error {
7883
impl From<DapiClientError> for Error {
7984
fn from(value: DapiClientError) -> Self {
8085
if let DapiClientError::Transport(TransportError::Grpc(status)) = &value {
86+
// If we have some consensus error metadata, we deserialize it and return as ConsensusError
8187
if let Some(consensus_error_value) = status
8288
.metadata()
8389
.get_bin("dash-serialized-consensus-error-bin")
@@ -88,11 +94,18 @@ impl From<DapiClientError> for Error {
8894
.map(|consensus_error| {
8995
Self::Protocol(ProtocolError::ConsensusError(Box::new(consensus_error)))
9096
})
91-
.unwrap_or_else(Self::Protocol);
97+
.unwrap_or_else(|e| {
98+
tracing::debug!("Failed to deserialize consensus error: {}", e);
99+
Self::Protocol(e)
100+
});
101+
}
102+
// Otherwise we parse the error code and act accordingly
103+
if status.code() == Code::AlreadyExists {
104+
return Self::AlreadyExists(status.message().to_string());
92105
}
93106
}
94107

95-
Self::DapiClientError(format!("{:?}", value))
108+
Self::DapiClientError(value.to_string())
96109
}
97110
}
98111

0 commit comments

Comments
 (0)