Skip to content
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Release channels have their own copy of this changelog:
* The `TransactionStatus`, `TransactionMemos`, and `AddressSignatures` columns
were updated in v1.18 to write a new key format. The old key format will no
no longer be supported for fallback reads as of v4.0
* `getSignaturesForAddress` returns an error with code `-32020` if the `before` or `until` signatures are not found, rather than a successful response with an empty array
#### Changes
* Added `--enable-scheduler-bindings` which binds an IPC server at `<ledger-path>/scheduler_bindings.ipc` for external schedulers to connect to.
* Added `clientId` field to each node in `getClusterNodes` response
Expand Down
14 changes: 10 additions & 4 deletions ledger/src/blockstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ type CompletedRanges = Vec<Range<u32>>;
pub struct SignatureInfosForAddress {
pub infos: Vec<ConfirmedTransactionStatusWithSignature>,
pub found_before: bool,
pub found_until: bool,
}

#[derive(Error, Debug)]
Expand Down Expand Up @@ -3555,20 +3556,24 @@ impl Blockstore {
// Generate a HashSet of signatures that should be excluded from the results based on
// `until` signature
let mut get_until_slot_timer = Measure::start("get_until_slot_timer");
let (lowest_slot, until_excluded_signatures) = match until {
None => (first_available_block, HashSet::new()),
let (lowest_slot, until_excluded_signatures, found_until) = match until {
None => (first_available_block, HashSet::new(), false),
Some(until) => {
let transaction_status =
self.get_transaction_status(until, &confirmed_unrooted_slots)?;
match transaction_status {
None => (first_available_block, HashSet::new()),
None => (first_available_block, HashSet::new(), false),
Some((slot, _)) => {
let mut slot_signatures = self.get_block_signatures_rev(slot)?;
if let Some(pos) = slot_signatures.iter().position(|&x| x == until) {
slot_signatures = slot_signatures.split_off(pos);
}

(slot, slot_signatures.into_iter().collect::<HashSet<_>>())
(
slot,
slot_signatures.into_iter().collect::<HashSet<_>>(),
true,
)
}
}
}
Expand Down Expand Up @@ -3678,6 +3683,7 @@ impl Blockstore {
Ok(SignatureInfosForAddress {
infos,
found_before: true, // if `before` signature was not found, this method returned early
found_until,
})
}

Expand Down
8 changes: 8 additions & 0 deletions rpc-client-api/src/custom_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub const JSON_RPC_SERVER_ERROR_MIN_CONTEXT_SLOT_NOT_REACHED: i64 = -32016;
pub const JSON_RPC_SERVER_ERROR_EPOCH_REWARDS_PERIOD_ACTIVE: i64 = -32017;
pub const JSON_RPC_SERVER_ERROR_SLOT_NOT_EPOCH_BOUNDARY: i64 = -32018;
pub const JSON_RPC_SERVER_ERROR_LONG_TERM_STORAGE_UNREACHABLE: i64 = -32019;
pub const JSON_RPC_SERVER_ERROR_FILTER_TRANSACTION_NOT_FOUND: i64 = -32020;

#[derive(Error, Debug)]
#[allow(clippy::large_enum_variant)]
Expand Down Expand Up @@ -80,6 +81,8 @@ pub enum RpcCustomError {
SlotNotEpochBoundary { slot: Slot },
#[error("LongTermStorageUnreachable")]
LongTermStorageUnreachable,
#[error("FilterTransactionNotFound")]
FilterTransactionNotFound { signature: String },
Comment on lines +84 to +85
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When using getTransaction with a signature that doesn't exist, we just return null rather than an error.

An error type makes more sense to me than returning null in this situation, so we'll end up doing something different here. Either way, we'll need to add the new error to kit as well, as the automated message mentions.

@steveluscher do you have a preference here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm fine with this being a JSON RPC error, but I wonder if it would be useful either:

  • Adding whether the not-found signature was in the before or until position
  • Making this two errors, one for before and one for until

It feels sort of bad making the developer guess what filter position the not-found signature was in, but I also don't know if it matters.

}

#[derive(Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -259,6 +262,11 @@ impl From<RpcCustomError> for Error {
message: "Failed to query long-term storage; please try again".to_string(),
data: None,
},
RpcCustomError::FilterTransactionNotFound { signature } => Self {
code: ErrorCode::ServerError(JSON_RPC_SERVER_ERROR_FILTER_TRANSACTION_NOT_FOUND),
message: format!("Transaction {signature} not found"),
data: None,
},
}
}
}
Expand Down
33 changes: 30 additions & 3 deletions rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1848,6 +1848,7 @@ impl JsonRpcRequestProcessor {
let SignatureInfosForAddress {
infos: mut results,
found_before,
found_until,
Comment thread
steviez marked this conversation as resolved.
} = self
.blockstore
.get_confirmed_signatures_for_address2(address, highest_slot, before, until, limit)
Expand All @@ -1874,7 +1875,7 @@ impl JsonRpcRequestProcessor {
.collect()
};

if results.len() < limit {
if results.len() < limit || !found_until {
if let Some(bigtable_ledger_storage) = &self.bigtable_ledger_storage {
let mut bigtable_before = before;
if !results.is_empty() {
Expand All @@ -1890,7 +1891,7 @@ impl JsonRpcRequestProcessor {
.get_signature_status(&bigtable_before.unwrap())
.await
{
Err(StorageError::SignatureNotFound) => {
Err(StorageError::SignatureNotFound(_)) => {
bigtable_before = None;
}
Err(err) => {
Expand Down Expand Up @@ -1924,12 +1925,38 @@ impl JsonRpcRequestProcessor {
}
}
}
Err(StorageError::SignatureNotFound) => {}
Err(StorageError::SignatureNotFound(not_found_signature)) => {
// bigtable_before is checked above
// SignatureNotFound means the blockstore before was not found, or the until signature was never found.
return Err(RpcCustomError::FilterTransactionNotFound {
signature: not_found_signature.to_string(),
}
.into());
}
Err(err) => {
warn!("Failed to query Bigtable: {err:?}");
return Err(RpcCustomError::LongTermStorageUnreachable.into());
}
}
} else {
// Long-term storage is not enabled.
// Return an error to the user if either before/until were provided but not found.
if !found_before {
if let Some(signature) = before {
Comment thread
steviez marked this conversation as resolved.
return Err(RpcCustomError::FilterTransactionNotFound {
signature: signature.to_string(),
}
.into());
}
}
if !found_until {
if let Some(signature) = until {
return Err(RpcCustomError::FilterTransactionNotFound {
signature: signature.to_string(),
}
.into());
}
}
}
}

Expand Down
10 changes: 5 additions & 5 deletions storage-bigtable/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub enum Error {
BlockNotFound(Slot),

#[error("Signature not found")]
SignatureNotFound,
SignatureNotFound(Signature),

#[error("tokio error")]
TokioJoinError(JoinError),
Expand Down Expand Up @@ -633,7 +633,7 @@ impl LedgerStorage {
.get_bincode_cell::<TransactionInfo>("tx", signature.to_string())
.await
.map_err(|err| match err {
bigtable::Error::RowNotFound => Error::SignatureNotFound,
bigtable::Error::RowNotFound => Error::SignatureNotFound(*signature),
_ => err.into(),
})?;
Ok(transaction_info.into())
Expand Down Expand Up @@ -713,7 +713,7 @@ impl LedgerStorage {
.get_bincode_cell("tx", signature.to_string())
.await
.map_err(|err| match err {
bigtable::Error::RowNotFound => Error::SignatureNotFound,
bigtable::Error::RowNotFound => Error::SignatureNotFound(*signature),
_ => err.into(),
})?;

Expand Down Expand Up @@ -772,7 +772,7 @@ impl LedgerStorage {
.get_bincode_cell("tx", before_signature.to_string())
.await
.map_err(|err| match err {
bigtable::Error::RowNotFound => Error::SignatureNotFound,
bigtable::Error::RowNotFound => Error::SignatureNotFound(*before_signature),
_ => err.into(),
})?;

Expand All @@ -788,7 +788,7 @@ impl LedgerStorage {
.get_bincode_cell("tx", until_signature.to_string())
.await
.map_err(|err| match err {
bigtable::Error::RowNotFound => Error::SignatureNotFound,
bigtable::Error::RowNotFound => Error::SignatureNotFound(*until_signature),
_ => err.into(),
})?;

Expand Down
Loading