feat(indexer)!: migrate to REST api + UTXO streaming - #1594
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReplaces the Indexer’s JSON-RPC interface with a new Axum-based REST API (with OpenAPI/Swagger), adds streaming UTXO updates, updates clients (Rust and JS) to REST, renames config/CLI fields from json_rpc to api, adjusts wallet SDK/services to the new client and streaming model, and updates tests, bindings, and CI accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client (Web/UI/SDK)
participant IDX as Indexer REST Server
participant Ctx as HandlerContext
participant SM as SubstateManager
participant TM as TransactionManager
participant NET as Networking
Note over C,IDX: REST replaces JSON-RPC
C->>IDX: GET /identity
IDX->>Ctx: Build HandlerContext
Ctx-->>IDX: public_key, peer_id, addresses
IDX-->>C: 200 { identity }
C->>IDX: GET /substates/{id}?version=&local_search_only=
IDX->>SM: get_substate(id, version)
alt Not found locally and !local_search_only
IDX->>TM: get_substate_from_network(requirement)
TM-->>IDX: SubstateResult (Up/Down/DoesNotExist)
end
IDX-->>C: 200/404 { substate or error }
C->>IDX: POST /transactions
IDX->>TM: submit_transaction or dry-run
TM-->>IDX: tx_id / result
IDX-->>C: 200 { tx_id/result }
sequenceDiagram
autonumber
participant WAL as Wallet SDK/Services
participant IDX as Indexer REST Server
participant STR as Stream Encoder
participant WALP as Wallet Scanner
Note over WAL,IDX: Streaming UTXO updates (per shard)
WAL->>IDX: POST /utxos/stream (resource, shard_state_versions, unspent_only)
IDX->>STR: Encode StartOfShard/Updates/EndOfShard
loop per shard batches
STR-->>WAL: bytes (length-delimited protobuf)
WAL->>WALP: decode -> UtxoUpdatePayload
WALP->>WALP: process SOS/updates/EOS, commit progress
end
STR-->>WAL: stream end
sequenceDiagram
autonumber
participant Node as Indexer
participant S as Server::spawn
participant Ax as Axum Router
participant OA as OpenAPI (utoipa)
participant UI as Swagger UI
Node->>S: spawn(preferred_addr, services, shutdown)
S->>Ax: build routes + layers (CORS, body limit)
S->>OA: generate OpenAPI
S->>UI: mount /swagger-ui and /openapi.json
S-->>Node: listening SocketAddr
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120–180 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
applications/tari_swarm_daemon/src/webserver/rpc/indexers.rs (1)
19-27: Update remainingjrpcreferences in the web UI
Replace all instances ofnode.jrpcwithnode.api_urlinapplications/tari_swarm_daemon/webui/src/routes/Main.tsx(e.g., lines 285, 288, 366–367).applications/tari_indexer/src/transaction_manager/mod.rs (1)
96-108: Update all remaining callers to the renamed method
Replace everyget_substate(...)call withget_substate_from_network(...), constructing aSubstateRequirementRef(e.g.SubstateRequirementRef::new(&id, version)) instead of separate address/version parameters and usingor_zero_version().to_substate_address()under the hood.
🧹 Nitpick comments (43)
crates/template_lib/src/models/utxo.rs (1)
98-100: LGTM! Method provides efficient byte access for serialization.The
as_bytes()method correctly returns a reference to the underlying 32-byte array, following Rust conventions for non-consuming accessors. This complements the existing API nicely, enabling efficient access for protobuf encoding and streaming scenarios without unnecessary copies.Optional: Consider adding a doc comment to clarify the method's purpose:
+ /// Returns a reference to the underlying 32-byte array representation of this UTXO ID. pub fn as_bytes(&self) -> &[u8; Self::LENGTH] { &self.0 }clients/tari_indexer_client/tests/streaming.rs (3)
8-10: Consider a more descriptive test name.The test name
dev_testdoesn't convey what is being tested. Consider renaming to something liketest_stream_utxo_updates_protobuf_integrationto make the test's purpose clearer.Apply this diff to improve the test name:
-async fn dev_test() { +async fn test_stream_utxo_updates_protobuf_integration() {
18-20: Consider documenting the test resource address.The hardcoded resource address could benefit from a comment explaining what it represents or why this specific address is used for testing.
Example:
+ // Test resource address - replace with actual resource when running against live indexer resource_address: "resource_0101010101010101010101010101010101010101010101010101010101010101" .parse() .unwrap(),
27-31: Add assertions to validate streaming behavior.The test only prints output without any assertions. Consider adding checks to validate expected behavior, such as:
- Verifying that messages are received (e.g.,
assert!(count > 0)after the loop)- Checking that
sos(start of stream) appears first- Validating that
eos(end of stream) appears last- Ensuring update messages have expected structure
This will help catch regressions in the streaming implementation.
Example:
let mut count = 0usize; + let mut saw_sos = false; + let mut saw_eos = false; while let Some(msg) = stream.try_next().await.unwrap() { count += 1; eprintln!("{count} {:?} {:?} {:?}", msg.sos, msg.update, msg.eos); + if msg.sos.is_some() { + assert!(!saw_sos, "Should only see one SOS message"); + saw_sos = true; + } + if msg.eos.is_some() { + saw_eos = true; + } } + assert!(saw_sos, "Should receive start-of-stream message"); + assert!(saw_eos, "Should receive end-of-stream message"); + assert!(count > 0, "Should receive at least one message");crates/common_types/src/array_utils.rs (2)
4-12: Add documentation for the public utility function.This function lacks documentation explaining its purpose, parameters, return value, and usage examples. Public APIs should be documented to help users understand how to use them correctly.
Consider adding documentation like:
+/// Attempts to copy exactly `SZ` bytes from `bytes` into a fixed-size array and convert to `T`. +/// +/// # Type Parameters +/// * `SZ` - The exact number of bytes required +/// * `T` - The target type that can be created from a `[u8; SZ]` array +/// +/// # Returns +/// * `Some(T)` if `bytes.len() == SZ` and conversion succeeds +/// * `None` if the length doesn't match +/// +/// # Examples +/// ``` +/// use tari_common_types::array_utils::copy_fixed_checked; +/// +/// let bytes = [1u8, 2, 3, 4]; +/// let result: Option<[u8; 4]> = copy_fixed_checked(&bytes); +/// assert!(result.is_some()); +/// +/// let too_short = [1u8, 2]; +/// let result: Option<[u8; 4]> = copy_fixed_checked(&too_short); +/// assert!(result.is_none()); +/// ``` pub fn copy_fixed_checked<const SZ: usize, T>(bytes: &[u8]) -> Option<T>
10-10: Simplify the slice operation.The slice operation
&bytes[..SZ]is redundant since line 6 already ensuresbytes.len() == SZ. You can passbytesdirectly tocopy_from_slice.Apply this diff:
- array.copy_from_slice(&bytes[..SZ]); + array.copy_from_slice(bytes);clients/javascript/indexer_client/tsconfig.json (1)
3-11: Align module resolution with ESM output.
moduleResolution: "node"keeps the legacy CommonJS resolver, so packages that rely onpackage.json"exports"(or dual ESM/CJS entrypoints) will fail to resolve during declaration builds. Because we emit ES modules (module: "ES2020"), switch to"NodeNext"or"Bundler"to match modern ESM semantics.- "moduleResolution": "node", + "moduleResolution": "NodeNext",applications/tari_swarm_daemon/webui/src/routes/Main.tsx (1)
291-296: LGTM! Consider making the label more descriptive.The implementation correctly follows the existing pattern for info sections and safely uses optional chaining to check for
api_url. The rendering placement is appropriate.Optional improvement: Consider using "REST API" instead of "API" for clarity, since the PR migrates from JSON-RPC to REST API:
- <b>API</b> + <b>REST API</b>clients/javascript/indexer_client/src/transports/index.ts (1)
22-24: Use camelCase for TypeScript property names.The property
timeout_millisuses snake_case, which is inconsistent with TypeScript/JavaScript naming conventions. TypeScript style guides recommend camelCase for property names.Apply this diff:
export interface TransportOptions { - timeout_millis?: number; + timeoutMillis?: number; }clients/javascript/indexer_client/src/transports/fetch.ts (2)
49-53: Update timeout_millis references to match camelCase convention.This references
timeout_milliswhich should be updated totimeoutMillisto match the TypeScript naming convention suggested for theTransportOptionsinterface.Apply this diff after updating the interface:
- const timeoutId = options?.timeout_millis + const timeoutId = options?.timeoutMillis ? setTimeout(() => { controller.abort("Timeout"); - }, options.timeout_millis) + }, options.timeoutMillis) : null;
59-68: Consider supporting array values in query parameters.The current implementation only handles primitive values and converts them to strings. If a query parameter is an array, it will be converted to a comma-separated string rather than being added as multiple parameters with the same key (e.g.,
?id=1&id=2), which is the standard URL encoding for arrays.If array query parameters are needed, apply this diff:
let query = null; if (typeof request.params === "object" && request.params !== null) { const urlParams = new URLSearchParams(); for (const [key, value] of Object.entries(request.params)) { if (value !== undefined && value !== null) { - urlParams.append(key, value.toString()); + if (Array.isArray(value)) { + value.forEach(v => urlParams.append(key, v.toString())); + } else { + urlParams.append(key, value.toString()); + } } } query = urlParams; }clients/javascript/indexer_client/src/index.ts (1)
59-113: Consider adding timeout configuration to API methods.While the transport layer supports timeouts via
TransportOptions, the API methods don't expose this capability to callers. For long-running operations or unreliable networks, users may want to configure per-request timeouts.Consider adding an optional
optionsparameter to methods:public submitTransaction( params: TransactionSubmitRequest, options?: TransportOptions ): Promise<TransactionSubmitResponse> { return this.transport.sendPost(`transactions`, params, options); }This would allow callers to specify timeouts when needed:
await client.submitTransaction(tx, { timeoutMillis: 30000 });applications/tari_swarm_daemon/src/process_definitions/context.rs (1)
109-130: Suggest extracting shared URL-resolution logic.This method duplicates the URL-resolution pattern from
get_public_json_rpc_url()(lines 84-107) andget_public_graphql_url()(lines 132-150). All three methods share the same public_ip derivation logic and URL construction approach, differing only in the setting key, port name, and error messages.Consider extracting a helper method to reduce duplication:
fn get_public_url( &self, setting_key: &str, port_name: &str, error_context: &str, ) -> Url { match self.settings.get(setting_key) { Some(url) => url.parse().unwrap_or_else(|_| panic!("Invalid {} URL", error_context)), None => { let public_ip = self .settings .get("public_ip") .map(|s| { if s == "127.0.0.1" { return "localhost"; } s.as_str() }) .unwrap_or("localhost"); let port = self .port_allocator .get(port_name) .unwrap_or_else(|| panic!("{} port must be allocated before calling get_public_url", port_name)); format!("http://{public_ip}:{port}") .parse() .unwrap_or_else(|_| panic!("Invalid {} URL", error_context)) }, } } pub fn get_public_api_url(&self) -> Url { self.get_public_url("public_api_url", "api", "API") } pub fn get_public_json_rpc_url(&self) -> Url { self.get_public_url("public_json_rpc_url", "jrpc", "JSON RPC") } pub fn get_public_graphql_url(&self) -> Url { self.get_public_url("public_graphql_url", "graphql", "GraphQL") }applications/tari_swarm_daemon/src/process_manager/handle.rs (1)
149-172: Consider refactoring shared URL-resolution logic.This method duplicates the URL-resolution pattern from
get_public_web_url(),get_public_json_rpc_url(), andget_public_graphql_url(). All methods share the same public_ip derivation and URL construction logic, differing only in setting keys, port names, and error messages.Consider extracting a helper method similar to the suggestion in
context.rsto reduce maintenance burden and ensure consistency across URL getters.crates/wallet/sdk/src/models/utxo_update.rs (1)
13-18: Derive serialization and TS exports; consider one-of enum.
- For consistency with adjacent types and cross-language use, add Serialize/Deserialize and TS export derives.
- Prefer a one-of enum to prevent invalid combinations (sos/update/eos simultaneously).
Apply derives:
-#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export))] pub struct UtxoUpdatePayload { pub sos: Option<StartOfShard>, pub update: Option<WalletUtxoUpdate>, pub eos: Option<EndOfShard>, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export))] pub struct StartOfShard { pub shard: Shard, pub max_state_version: StateVersion, pub has_more: bool, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export))] pub struct EndOfShard { pub max_state_version: StateVersion, }Optional enum shape:
pub enum UtxoUpdatePayload { Start(StartOfShard), Update(WalletUtxoUpdate), End(EndOfShard), }Also applies to: 20-26, 27-30
applications/tari_indexer/src/rest_api/streaming/encoding.rs (3)
10-11: Avoid server→client dependency for protobuf types.Importing protobuf types from the client crate couples layering. Prefer a shared crate for proto definitions consumed by both server and client.
Example:
- Create tari_indexer_proto (prost-generated types).
- Depend on it from both applications/tari_indexer and clients/tari_indexer_client.
23-31: Reduce log noise and redundant work.Compute encoded_len once and avoid always logging at debug on hot paths.
- if log_enabled!(Level::Trace) { - let len = msg.encoded_len(); - trace!(target: LOG_TARGET, "🚧 Encoding protobuf message of length: {}", len); - } - let len = msg.encoded_len(); - debug!(target: LOG_TARGET, "🚧 Encoding protobuf message of length: {}", len); + let len = msg.encoded_len(); + if log_enabled!(Level::Trace) { + trace!(target: LOG_TARGET, "Encoding protobuf message of length: {}", len); + } msg.encode_length_delimited(buf)?;
70-88: Clarify Accept parsing and broaden protobuf media type (optional).
- Parameter name suggests a single MIME type; the function parses an Accept header. Consider renaming for clarity.
- Optionally accept application/protobuf in addition to application/x-protobuf.
-pub fn from_media_type(mime_type: &str) -> Option<MimeTypeEncoder> { +pub fn from_accept_header(accept: &str) -> Option<MimeTypeEncoder> { @@ - let media_type = headers_accept::Accept::from_str(mime_type).ok()?; + let media_type = headers_accept::Accept::from_str(accept).ok()?; @@ - MediaType::new(Name::new_unchecked("application"), Name::new_unchecked("x-protobuf")), + MediaType::new(Name::new_unchecked("application"), Name::new_unchecked("x-protobuf")), + MediaType::new(Name::new_unchecked("application"), Name::new_unchecked("protobuf")),Also consider returning a Result to map unsupported types to HTTP 406 upstream.
integration_tests/src/wallet_daemon.rs (1)
70-75: Renameindexer_urltoindexer_api_urland consider readiness wait.
- Rename the local variable to match the config field:
- let indexer_url = format!("http://127.0.0.1:{}", indexer_api_port); + let indexer_api_url = format!("http://127.0.0.1:{}", indexer_api_port); - config.ootle_wallet_daemon.indexer_api_url = indexer_url.parse().unwrap(); + config.ootle_wallet_daemon.indexer_api_url = indexer_api_url.parse().unwrap();
- Optionally wait for the indexer API to accept connections before spawning the wallet daemon to avoid startup races.
applications/tari_indexer/web_ui/src/utils/json_rpc.tsx (1)
83-99: Add error handling for failed client initialization.The singleton pattern does not handle initialization failures gracefully. If
getClientAddress()or client creation fails, the error will propagate to the caller, butclientInstanceremainsnulland subsequent calls will retry indefinitely. Consider adding error state management or a retry limit.Apply this diff to add basic error recovery:
let clientInstance: IndexerClient | null = null; let pendingClientInstance: Promise<IndexerClient> | null = null; let outerAddress: URL | null = null; +let initializationFailed = false; export async function client() { if (clientInstance) { return Promise.resolve(clientInstance); } + if (initializationFailed) { + throw new Error("Client initialization previously failed. Reload the page to retry."); + } const getAddress = outerAddress ? Promise.resolve(outerAddress) : getClientAddress(); pendingClientInstance = getAddress.then(async (addr) => { const client = IndexerClient.usingFetchTransport(addr.toString()); outerAddress = addr; clientInstance = client; pendingClientInstance = null; return client; - }); + }).catch((err) => { + initializationFailed = true; + pendingClientInstance = null; + throw err; + }); return pendingClientInstance; }crates/wallet/sdk_services/src/indexer_rest_api.rs (2)
172-241: Review streaming implementation for robustness and magic number usage.The new UTXO streaming implementation looks well-structured overall, but has a few concerns:
- Magic number
1000appears in two places (line 184 and 193) forper_shard_limitandhas_morethreshold. Consider extracting this as a named constant.- Fixed array assumptions: Lines 211 and 222 use
copy_fixed_checkedwhich could fail silently if array sizes don't match. The error handling is present but the underlying assumption should be documented.- Protobuf decoding error handling: The nested pattern matching and error conversion (lines 190-239) is thorough but complex. Consider extracting helper functions for each variant decoding to improve readability.
Apply this diff to address the magic number:
+const DEFAULT_PER_SHARD_LIMIT: u32 = 1000; + impl WalletNetworkInterface for IndexerRestApiNetworkInterface { // ... async fn stream_stealth_utxo_updates( &self, resource_address: ResourceAddress, shard_state_versions: Vec<(Shard, StateVersion)>, unspent_only: bool, ) -> Result<UtxoUpdateStream<Self::Error>, Self::Error> { let mut client = self.get_client()?; let stream = client .stream_utxo_updates_protobuf(GetUtxoUpdatesRequest { shard_state_versions, resource_address, unspent_only, - per_shard_limit: 1000, + per_shard_limit: DEFAULT_PER_SHARD_LIMIT, }) .await?; let stream = stream .map_err(|e| IndexerRestApiNetworkInterfaceError::StreamDecodeError(e.into())) .and_then(|res| async move { let sos = res.sos.map(|sos| StartOfShard { shard: Shard::from(sos.shard), max_state_version: StateVersion::from(sos.max_state_version), - has_more: sos.num_updates >= 1000, + has_more: sos.num_updates >= DEFAULT_PER_SHARD_LIMIT, });For the decoding complexity, consider extracting helper functions:
fn decode_unspent(unspent: protobuf::UtxoUnspent) -> Result<UtxoUnspent, IndexerRestApiNetworkInterfaceError> { let public_nonce = RistrettoPublicKeyBytes::from_bytes(&unspent.public_nonce) .map_err(|e| IndexerRestApiNetworkInterfaceError::StreamDecodeError(anyhow!("Failed to decode public nonce: {e}")))?; Ok(UtxoUnspent { tag: unspent.tag.into(), public_nonce, }) } fn decode_spent(spent: protobuf::UtxoSpent) -> Result<UtxoSpent, IndexerRestApiNetworkInterfaceError> { let id_arr = copy_fixed_checked(&spent.id) .ok_or_else(|| IndexerRestApiNetworkInterfaceError::StreamDecodeError(anyhow!("Failed to decode UTXO ID, incorrect length")))?; Ok(UtxoSpent { id: UtxoId::from_array(id_arr), version: spent.version, }) } fn decode_burnt(burnt: protobuf::UtxoBurnt) -> Result<UtxoBurnt, IndexerRestApiNetworkInterfaceError> { let id_arr = copy_fixed_checked(&burnt.id) .ok_or_else(|| IndexerRestApiNetworkInterfaceError::StreamDecodeError(anyhow!("Failed to decode UTXO ID, incorrect length")))?; Ok(UtxoBurnt { id: UtxoId::from_array(id_arr), version: burnt.version, }) }Then simplify the match:
let update = res.update .map(|u| match u { protobuf::WalletUtxoUpdate::Unspent(unspent) => decode_unspent(unspent).map(WalletUtxoUpdate::Unspent), protobuf::WalletUtxoUpdate::Spent(spent) => decode_spent(spent).map(WalletUtxoUpdate::Spent), protobuf::WalletUtxoUpdate::Burnt(burnt) => decode_burnt(burnt).map(WalletUtxoUpdate::Burnt), }) .transpose()?;
99-114: Verify error message accuracy for client-side validation.Lines 103-108 construct an
IndexerRestClientError::RequestFailedWithStatuserror whensubstate_idscannot be converted to the required type. The comment states this simulates what the indexer would return, but this is a client-side validation failure that occurs before any network request.Consider using a more specific error variant or clarifying the comment:
let resp = client .fetch_substates(GetSubstatesRequest { requests: substate_ids.try_into().map_err(|_| { - // We can't send the request, but return an error that would be the same/similar to what the indexer - // would return + // Client-side validation: too many substate IDs requested (exceeds protocol limit) IndexerRestApiNetworkInterfaceError::IndexerClientError( IndexerRestClientError::RequestFailedWithStatus { code: INVALID_REQUEST_CODE, message: "Too many substate IDs requested".to_string(), }, ) })?, }) .await?;bindings/src/types/tari-indexer-client/IndexerGetIdentityResponse.ts (1)
7-7: LGTM!The array type syntax
string[]is more concise and idiomatic thanArray<string>. Both are functionally equivalent.applications/tari_indexer/src/storage_sqlite/store_factory.rs (1)
179-191: Document and consider replacing boolean flag with an enum
- Update the doc to explain unspents_only semantics.
- Prefer an explicit filter enum to avoid boolean ambiguity, e.g.,
pub enum SpentFilter { All, UnspentOnly }and
fn utxos_get_updates(&mut self, ..., spent_filter: SpentFilter, limit: u32) -> ...applications/tari_indexer/src/rest_api/server.rs (3)
28-46: Add missing OpenAPI path for non-fungibles endpointThe route is exposed but not documented. Include it in ApiDoc.
#[derive(OpenApi)] #[openapi(paths( @@ - handlers::utxos::stream_utxo_updates, + handlers::utxos::stream_utxo_updates, + handlers::nfts::get_non_fungibles, ))] pub struct ApiDoc;Also applies to: 87-87
90-92: Tighten CORS configurationCorsLayer::permissive() is broad. Prefer allowing specific origins/methods from config/env.
Example (sketch):
// CorsLayer::new().allow_origin(AllowOrigin::list([...])).allow_methods([Method::GET, Method::POST])
102-103: Fix log message textMessage references “Wallet query”; should mention “Indexer REST API”.
- error!(target: LOG_TARGET, "Wallet query HTTP server error: {error}"); + error!(target: LOG_TARGET, "Indexer REST API server error: {error}");applications/tari_indexer/src/bootstrap.rs (1)
226-229: Use CommonConfig accessor for base pathFor consistency and to avoid relying on field visibility, prefer the accessor:
- let substate_cache_dir = config.common.base_path.join("substate_cache"); + let substate_cache_dir = config.common.base_path().join("substate_cache");applications/tari_walletd/src/cli.rs (1)
55-56: Add environment variable support for the indexer API URLIn
applications/tari_walletd/src/cli.rs, update theclapattribute forindexer_api_url:- #[clap(long, short = 'i', alias = "indexer-url")] + #[clap(long, short = 'i', alias = "indexer-url", env = "TARI_WALLET_INDEXER_API_URL")]This enables setting the indexer API URL via the
TARI_WALLET_INDEXER_API_URLenvironment variable.applications/tari_indexer/src/substate_manager.rs (2)
118-125: Confirm unspent_only semantics won’t desync clientsFiltering updates to only unspent UTXOs means Spent/Burnt events after from_state_version are omitted. Wallets relying on this endpoint might miss state transitions and keep stale UTXOs unless they query another path. Please confirm this is intended for initial snapshots only or document the contract; otherwise, consider returning all updates and letting clients filter.
149-162: Minor: align parameter naming to substate_id for consistencyget_substate uses substate_id; get_substate_from_db still uses substate_address. Consider renaming for consistency.
- async fn get_substate_from_db( - &self, - substate_address: &SubstateId, - version: Option<u32>, - ) -> Result<Option<SubstateResponse>, anyhow::Error> { + async fn get_substate_from_db( + &self, + substate_id: &SubstateId, + version: Option<u32>, + ) -> Result<Option<SubstateResponse>, anyhow::Error> { let mut tx = self.substate_store.create_read_tx()?; - if let Some(row) = tx.get_substate(substate_address, version)? { + if let Some(row) = tx.get_substate(substate_id, version)? {Also applies to: 181-188
applications/tari_indexer/src/storage_sqlite/reader.rs (1)
398-411: Unspent-only filtering: verify end-to-end behavior; small query clarity refactor
- Behavior: With unspent_only=true, Spent/Burnt updates are excluded. Ensure streaming consumers don’t rely on those transitions to reconcile state; otherwise they may retain spent UTXOs. Consider documenting or adding an “all updates” mode as default.
- Clarity: Build the boxed query first, then apply conditional filters, and finally order/limit for readability (no semantic change).
- let mut query = utxos::table - .filter(utxos::resource_address.eq(resource_address.to_string())) - .filter(utxos::state_version.gt(from_state_version.as_u64() as i64)) - .filter(utxos::shard.eq(shard.as_u32() as i32)) - .limit(i64::from(limit)) - .order_by(utxos::state_version.asc()) - .into_boxed(); + let mut query = utxos::table + .filter(utxos::resource_address.eq(resource_address.to_string())) + .filter(utxos::state_version.gt(from_state_version.as_u64() as i64)) + .filter(utxos::shard.eq(shard.as_u32() as i32)) + .into_boxed(); if unspent_only { // Only return unspent UTXOs - query = query - .filter(utxos::is_spent.eq(false)) - .filter(utxos::is_burnt.eq(false)); + query = query.filter(utxos::is_spent.eq(false)).filter(utxos::is_burnt.eq(false)); } + let query = query.order_by(utxos::state_version.asc()).limit(i64::from(limit));clients/tari_indexer_client/src/error.rs (1)
38-58: REST error enum looks good; minor API polish suggestionEnum and IsNotFoundError impl are sound. Consider using StatusCode for RequestFailedWithStatus.code to avoid i64 casting and to align with reqwest.
- RequestFailedWithStatus { code: i64, message: String }, + RequestFailedWithStatus { code: reqwest::StatusCode, message: String },Note: adjust constructors/callers accordingly.
Also applies to: 60-70
clients/tari_indexer_client/src/rest_api/types.rs (1)
8-21: TS/serde compatibility for fields
- Add ts(type = "string") on public_key to avoid requiring TS on external type.
- Ensure multiaddr is compiled with its "serde" feature; otherwise Serialize/Deserialize for Multiaddr will fail.
Apply this small TS tweak:
pub struct GetIdentityResponse { pub peer_id: String, - #[cfg_attr(feature = "utoipa", schema(value_type = String))] + #[cfg_attr(feature = "utoipa", schema(value_type = String))] + #[cfg_attr(feature = "ts", ts(type = "string"))] pub public_key: RistrettoPublicKeyBytes, #[cfg_attr(feature = "utoipa", schema(value_type = Vec<String>))] #[cfg_attr(feature = "ts", ts(type = "string[]"))] pub public_addresses: Vec<Multiaddr>, }Also please confirm that the multiaddr dependency has the "serde" feature enabled in this crate.
applications/tari_indexer/src/cli.rs (1)
46-49: Preserve CLI compatibility with old flag/env namesConsider keeping the old CLI flag as an alias for a smoother migration:
- Add alias "json-rpc-address" to api_listen_address.
- Web UI env var was renamed; if you want backward compatibility, add a hidden fallback reader for TARI_INDEXER_WEB_UI_PUBLIC_JSON_RPC_URL (optional).
Apply this alias for the CLI flag:
/// Bind address for API server -#[clap(long, short = 'r', alias = "api-address")] +#[clap(long, short = 'r', alias = "api-address")] +#[clap(alias = "json-rpc-address")] // backward-compat pub api_listen_address: Option<SocketAddr>,Also applies to: 64-66
applications/tari_indexer/src/rest_api/handlers/misc.rs (1)
7-11: Layering: server depends on client crate typesThe handler returns tari_indexer_client::rest_api::types::GetIdentityResponse. This inverts dependency direction (server → client). Prefer moving shared REST DTOs into a neutral crate (e.g., tari_indexer_types) consumed by both server and client.
Also applies to: 23-35
applications/tari_indexer/src/rest_api/handlers/utxos.rs (2)
60-69: Use typed header constant; keep behaviorMinor: prefer the typed header constant for case-insensitive lookup. Optionally consider 406 Not Acceptable instead of 400 for unsupported Accept, if your ErrorResponse supports it.
Apply this tweak:
- let accept_header = headers.get("accept").and_then(|v| v.to_str().ok()); + let accept_header = headers + .get(axum::http::header::ACCEPT) + .and_then(|v| v.to_str().ok());
29-33: Deduplicate magic numbers for limitsExtract the “1000” limits into named constants to keep them in sync and self-document intent.
const LOG_TARGET: &str = "tari::ootle::indexer::rest_api::handlers::utxos"; +const MAX_PER_SHARD_LIMIT: u32 = 1000; +const MAX_FETCH_UTXOS: usize = 1000; @@ - if req.per_shard_limit > 1000 { + if req.per_shard_limit > MAX_PER_SHARD_LIMIT { return Err(ErrorResponse::bad_request( - "per_shard_limit cannot be greater than 1000", + "per_shard_limit cannot be greater than 1000", )); } @@ - if req.tag_and_nonce_pairs.len() > 1000 { + if req.tag_and_nonce_pairs.len() > MAX_FETCH_UTXOS { return Err(ErrorResponse::bad_request("cannot query more than 1000 UTXOs")); }Also applies to: 84-86
applications/tari_indexer/src/rest_api/error.rs (1)
9-13: Verify utoipa Schema import pathEnsure the Schema path matches the utoipa version in use. In utoipa 5.x, Schema typically lives under openapi::schema::Schema.
If needed:
-use utoipa::{ - openapi::{RefOr, Schema}, - PartialSchema, - ToSchema, -}; +use utoipa::{openapi::RefOr, PartialSchema, ToSchema}; +use utoipa::openapi::schema::Schema;Please confirm compilation against the current utoipa version. Based on learnings
clients/tari_indexer_client/src/protobuf.rs (2)
4-12: Add Clone/PartialEq for ergonomic usage and testsDeriving Clone and PartialEq (and Eq where applicable) eases testing and handling in streams.
-#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct UtxoUpdatePayload { ... } -#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct StartOfShard { ... } -#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct EndOfShard { ... } -#[derive(::prost::Oneof, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Oneof, serde::Serialize, serde::Deserialize)] pub enum WalletUtxoUpdate { ... } -#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct UtxoUnspent { ... } -#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct UtxoSpent { ... } -#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct UtxoBurnt { ... }Also applies to: 14-22, 24-62
40-46: Optional: use bytes::Bytes for zero‑copy protobuf bytesIf prost is configured to support bytes::Bytes, prefer it over Vec to reduce allocations in high‑throughput streams.
Example:
-#[derive(::prost::Message, serde::Serialize, serde::Deserialize)] +#[derive(Clone, PartialEq, ::prost::Message, serde::Serialize, serde::Deserialize)] pub struct UtxoUnspent { #[prost(uint32, tag = "1")] pub tag: u32, - #[prost(bytes, tag = "2")] - pub public_nonce: Vec<u8>, + #[prost(bytes, tag = "2")] + pub public_nonce: bytes::Bytes, }Repeat for id fields in UtxoSpent/UtxoBurnt. Ensure serde support (serde_bytes or custom) if JSON encoding is required. Based on learnings
Also applies to: 48-54, 56-62
applications/tari_indexer/src/rest_api/handlers/transactions.rs (1)
115-120: Consider GET for idempotent retrievalget_transaction_result is a read; prefer GET over POST for REST semantics and cache/proxy friendliness.
If routing is updated elsewhere, adjust the utoipa path to:
-#[utoipa::path( - post, +#[utoipa::path( + get, path = "/transactions/{transaction_id}/result",applications/tari_indexer/src/rest_api/context.rs (1)
36-50: Make cache-control flag configurablecache_control_enabled is hardcoded to true. Source this from configuration (Services or env) to enable/disable caching per deployment.
- cache_control_enabled: true, + cache_control_enabled: services.config.http_cache_enabled,Or expose a setter on HandlerContext if config plumbed later.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (4)
applications/tari_indexer/src/rest_api/error.rs (1)
90-94: Fromanyhow::Error now masks details to clientsMapping via anyhow() → internal_error() masks messages by default. Good improvement over previous behavior.
applications/tari_indexer/src/rest_api/handlers/nfts.rs (1)
14-19: Good: explicit validation for end_index/start_index orderingClear 400 on invalid ranges improves UX vs silent saturation.
applications/tari_indexer/src/rest_api/handlers/templates.rs (1)
77-86: Good: optional limit handled with sensible defaultUsing Option + unwrap_or avoids 400s on missing limit and matches the documented optional param.
applications/tari_indexer/src/rest_api/handlers/transactions.rs (1)
89-92: Correct use of time 0.3: OffsetDateTime::now_utc()The helper now uses OffsetDateTime, which is the right API; good fix.
🧹 Nitpick comments (11)
clients/tari_indexer_client/tests/streaming.rs (2)
8-10: Improve test naming to describe the tested behavior.The name
dev_testdoesn't convey what functionality is being tested. Consider renaming to something more descriptive liketest_stream_utxo_updates_all_shardsortest_stream_protobuf_utxo_updates_manual.Apply this diff to improve the test name:
-async fn dev_test() { +async fn test_stream_utxo_updates_protobuf_manual() {
11-25: Enhance error messages withexpect()instead ofunwrap().Multiple
unwrap()calls will panic without providing useful context when failures occur. Consider usingexpect()with descriptive messages to make debugging easier.Apply this diff to improve error handling:
- let mut client = IndexerRestApiClient::connect("http://localhost:12017").unwrap(); + let mut client = IndexerRestApiClient::connect("http://localhost:12017") + .expect("Failed to connect to indexer at localhost:12017"); let mut stream = client .stream_utxo_updates_protobuf(GetUtxoUpdatesRequest { shard_state_versions: NumPreshards::current() .all_shards_iter() .map(|shard| (shard, StateVersion::zero())) .collect(), - resource_address: "resource_0101010101010101010101010101010101010101010101010101010101010101" - .parse() - .unwrap(), + resource_address: "resource_0101010101010101010101010101010101010101010101010101010101010101" + .parse() + .expect("Failed to parse test resource address"), unspent_only: false, per_shard_limit: 1000, }) .await - .unwrap(); + .expect("Failed to create UTXO update stream");Optional: Extract hardcoded values to constants.
For improved maintainability, consider extracting the hardcoded URL and test resource address to named constants at the top of the test.
const TEST_INDEXER_URL: &str = "http://localhost:12017"; const TEST_RESOURCE_ADDRESS: &str = "resource_0101010101010101010101010101010101010101010101010101010101010101";applications/tari_indexer/web_ui/src/utils/json_rpc.tsx (2)
51-51: Remove unused WalletDaemonClient import.The
WalletDaemonClientimport is no longer used after migrating to the REST API client.Apply this diff to remove the unused import:
-import { WalletDaemonClient } from "@tari-project/wallet_jrpc_client"; import { IndexerClient } from "@tari-project/indexer_client";
54-59: Remove duplicate environment variable check.Line 56 and line 57 both check
import.meta.env.VITE_API_ADDRESS, making one of them redundant.Apply this diff to remove the duplicate:
const DEFAULT_API_ADDRESS = new URL( import.meta.env.VITE_INDEXER_API_ADDRESS || - import.meta.env.VITE_API_ADDRESS || import.meta.env.VITE_API_ADDRESS || "http://localhost:9000", );applications/tari_indexer/src/lib.rs (1)
112-118: REST API spawning looks correct.The server is spawned correctly, the actual bound address is captured, and errors are propagated appropriately.
Consider upgrading the log level to
info!for consistency with the GraphQL server logging at Line 108:- debug!(target: LOG_TARGET, "API address {}", listen_address); + info!(target: LOG_TARGET, "🌐 REST API listening on {}", listen_address);clients/tari_indexer_client/src/protobuf_stream.rs (1)
42-46: Consider adding an inline comment explaining the varint-completion check.The logic
tmp_slice.len() < 10 && tmp_slice.iter().take(10).all(|byte| byte & 0x80 != 0)is correct but subtle. An inline comment clarifying that a varint is complete when any byte has MSB = 0, and that we continue buffering only if all available bytes (fewer than 10) have MSB = 1, would aid future maintainers.Example:
let tmp_slice = &this.buf[..]; - // A length-delimited varint is complete once a byte with MSB 0 is seen (max 10 bytes for u64). + // A protobuf varint is complete when a byte with MSB = 0 appears (max 10 bytes for u64). + // If we have <10 bytes and all have MSB = 1, the varint is incomplete—continue buffering. + // If we have ≥10 bytes all with MSB = 1, that's invalid and decode_length_delimiter will fail. if tmp_slice.len() < 10 && tmp_slice.iter().take(10).all(|byte| byte & 0x80 != 0) {applications/tari_indexer/src/rest_api/error.rs (1)
46-46: Avoid duplicate error logsanyhow() and internal_error() both log; each anyhow-based error logs twice. Log in one place only.
Option: remove the log in anyhow() and keep it in internal_error(), or gate the second log behind a flag to prevent duplicates.
Also applies to: 53-53
applications/tari_indexer/src/rest_api/handlers/nfts.rs (1)
20-21: Drop saturating_sub after ordering checkWith end_index >= start_index validated, plain subtraction is simpler and avoids surprising 0 from saturation logic.
Apply:
- let limit = usize::try_from(req.end_index.saturating_sub(req.start_index)) + let limit = usize::try_from(req.end_index - req.start_index) .map_err(|e| ErrorResponse::bad_request(format!("Invalid end_index: {}", e)))?;Also, please confirm if end_index is exclusive or inclusive to avoid off-by-one issues, and document accordingly.
applications/tari_indexer/src/rest_api/handlers/templates.rs (1)
56-59: Minor: fix comment typo“ad” → “as”.
- // TemplateExecutable::DownloadableWasm is never returned ad there is no DB type for that + // TemplateExecutable::DownloadableWasm is never returned as there is no DB type for thatapplications/tari_indexer/src/rest_api/handlers/transactions.rs (2)
34-38: Consider GET for a read-only “result” endpointFetching a result is idempotent/read-only; GET better matches semantics and client/proxy expectations.
Change utoipa path to get and update router accordingly.
71-73: Don’t leak internal error details in 503 messageIncluding the full error string may expose internals. Return a generic message to clients; keep details in logs.
- ErrorResponse::service_unavailable(format!("All validators failed: {}", e)) + ErrorResponse::service_unavailable(if cfg!(debug_assertions) { + format!("All validators failed: {}", e) + } else { + "All validators failed".to_string() + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
applications/tari_indexer/README.md(1 hunks)applications/tari_indexer/src/lib.rs(4 hunks)applications/tari_indexer/src/rest_api/error.rs(1 hunks)applications/tari_indexer/src/rest_api/handlers/nfts.rs(1 hunks)applications/tari_indexer/src/rest_api/handlers/templates.rs(1 hunks)applications/tari_indexer/src/rest_api/handlers/transactions.rs(1 hunks)applications/tari_indexer/web_ui/src/utils/json_rpc.tsx(2 hunks)applications/tari_swarm_daemon/src/process_definitions/indexer.rs(2 hunks)applications/tari_walletd/src/config.rs(2 hunks)bindings/src/types/tari-indexer-client/GetUtxoUpdatesRequest.ts(1 hunks)bindings/src/types/tari-indexer-client/IndexerGetSubstateRequest.ts(1 hunks)bindings/src/types/tari-indexer-client/ListTemplatesRequest.ts(1 hunks)clients/javascript/indexer_client/README.md(1 hunks)clients/javascript/indexer_client/package.json(1 hunks)clients/javascript/indexer_client/src/index.ts(1 hunks)clients/javascript/indexer_client/src/transports/fetch.ts(1 hunks)clients/javascript/wallet_daemon_client/package.json(1 hunks)clients/javascript/wallet_daemon_client/src/transports/fetch.ts(2 hunks)clients/tari_indexer_client/src/protobuf_stream.rs(1 hunks)clients/tari_indexer_client/src/types.rs(7 hunks)clients/tari_indexer_client/tests/streaming.rs(1 hunks)crates/state_store_rocksdb/src/utils.rs(2 hunks)crates/storage/src/global/template_db.rs(2 hunks)crates/template_manager/src/implementation/manager.rs(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- applications/tari_indexer/README.md
- clients/javascript/wallet_daemon_client/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- clients/javascript/indexer_client/package.json
- clients/javascript/indexer_client/src/transports/fetch.ts
- clients/javascript/indexer_client/README.md
🧰 Additional context used
🧬 Code graph analysis (12)
clients/tari_indexer_client/tests/streaming.rs (2)
bindings/src/types/tari-indexer-client/GetUtxoUpdatesRequest.ts (1)
GetUtxoUpdatesRequest(6-11)bindings/src/types/NumPreshards.ts (1)
NumPreshards(3-3)
applications/tari_swarm_daemon/src/process_definitions/indexer.rs (1)
applications/tari_swarm_daemon/src/process_definitions/context.rs (1)
listen_ip(156-158)
applications/tari_indexer/src/rest_api/handlers/templates.rs (6)
bindings/src/types/tari-indexer-client/GetTemplateDefinitionResponse.ts (1)
GetTemplateDefinitionResponse(4-4)bindings/src/types/tari-indexer-client/ListTemplatesRequest.ts (1)
ListTemplatesRequest(3-3)bindings/src/types/tari-indexer-client/ListTemplatesResponse.ts (1)
ListTemplatesResponse(4-4)bindings/src/types/tari-indexer-client/TemplateMetadata.ts (1)
TemplateMetadata(4-11)applications/tari_indexer/src/rest_api/error.rs (4)
not_found(66-71)internal_error(51-63)bad_request(74-79)anyhow(44-48)crates/engine/src/wasm/module.rs (2)
code(102-104)from_code(61-63)
applications/tari_indexer/src/rest_api/handlers/nfts.rs (2)
bindings/src/types/tari-indexer-client/GetNonFungiblesRequest.ts (1)
GetNonFungiblesRequest(4-4)applications/tari_indexer/src/rest_api/error.rs (2)
bad_request(74-79)anyhow(44-48)
applications/tari_indexer/web_ui/src/utils/json_rpc.tsx (14)
clients/javascript/indexer_client/src/index.ts (6)
IndexerClient(40-118)getConnections(79-81)listSubstates(91-93)getNonFungibles(83-85)getTransactionResult(103-105)listRecentTransactions(107-109)bindings/src/types/tari-indexer-client/IndexerGetIdentityResponse.ts (1)
IndexerGetIdentityResponse(4-8)bindings/src/types/tari-indexer-client/IndexerAddPeerRequest.ts (1)
IndexerAddPeerRequest(4-8)bindings/src/types/tari-indexer-client/IndexerAddPeerResponse.ts (1)
IndexerAddPeerResponse(3-3)bindings/src/types/tari-indexer-client/IndexerGetConnectionsResponse.ts (1)
IndexerGetConnectionsResponse(4-4)bindings/src/types/tari-indexer-client/IndexerGetSubstateRequest.ts (1)
IndexerGetSubstateRequest(3-3)bindings/src/types/tari-indexer-client/IndexerGetSubstateResponse.ts (1)
IndexerGetSubstateResponse(4-4)bindings/src/types/tari-indexer-client/ListSubstatesRequest.ts (1)
ListSubstatesRequest(5-10)bindings/src/types/tari-indexer-client/ListSubstatesResponse.ts (1)
ListSubstatesResponse(4-4)bindings/src/types/tari-indexer-client/GetNonFungiblesRequest.ts (1)
GetNonFungiblesRequest(4-4)bindings/src/types/tari-indexer-client/IndexerGetTransactionResultRequest.ts (1)
IndexerGetTransactionResultRequest(4-4)bindings/src/types/tari-indexer-client/IndexerGetTransactionResultResponse.ts (1)
IndexerGetTransactionResultResponse(4-4)bindings/src/types/tari-indexer-client/ListRecentTransactionsRequest.ts (1)
ListRecentTransactionsRequest(4-4)bindings/src/types/tari-indexer-client/ListRecentTransactionsResponse.ts (1)
ListRecentTransactionsResponse(4-4)
crates/storage/src/global/template_db.rs (3)
applications/tari_indexer/src/rest_api/handlers/transactions.rs (1)
now(89-92)crates/state_store_rocksdb/src/utils.rs (1)
now(43-46)crates/template_manager/src/implementation/manager.rs (1)
now(441-444)
applications/tari_indexer/src/lib.rs (2)
applications/tari_indexer/src/graphql/server.rs (1)
run_graphql(53-75)applications/tari_indexer/src/substate_manager.rs (1)
new(79-91)
clients/tari_indexer_client/src/types.rs (2)
bindings/src/types/Shard.ts (1)
Shard(3-3)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)
applications/tari_indexer/src/rest_api/handlers/transactions.rs (7)
bindings/src/types/Decision.ts (1)
Decision(4-4)bindings/src/types/tari-indexer-client/IndexerTransactionFinalizedResult.ts (1)
IndexerTransactionFinalizedResult(5-15)bindings/src/types/tari-indexer-client/ListRecentTransactionsRequest.ts (1)
ListRecentTransactionsRequest(4-4)bindings/src/types/tari-indexer-client/ListRecentTransactionsResponse.ts (1)
ListRecentTransactionsResponse(4-4)bindings/src/types/TransactionId.ts (1)
TransactionId(3-3)applications/tari_indexer/src/rest_api/context.rs (1)
transaction_manager(76-81)applications/tari_indexer/src/rest_api/error.rs (4)
anyhow(44-48)service_unavailable(82-87)bad_request(74-79)not_found(66-71)
crates/state_store_rocksdb/src/utils.rs (3)
applications/tari_indexer/src/rest_api/handlers/transactions.rs (1)
now(89-92)crates/storage/src/global/template_db.rs (1)
now(120-123)crates/template_manager/src/implementation/manager.rs (1)
now(441-444)
crates/template_manager/src/implementation/manager.rs (3)
applications/tari_indexer/src/rest_api/handlers/transactions.rs (1)
now(89-92)crates/state_store_rocksdb/src/utils.rs (1)
now(43-46)crates/storage/src/global/template_db.rs (1)
now(120-123)
clients/javascript/indexer_client/src/index.ts (2)
clients/javascript/indexer_client/src/transports/index.ts (1)
HttpTransport(10-20)clients/javascript/indexer_client/src/transports/fetch.ts (1)
FetchTransport(8-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: check nightly
- GitHub Check: test
- GitHub Check: check stable
- GitHub Check: file licenses
- GitHub Check: machete
- GitHub Check: fmt
- GitHub Check: clippy
🔇 Additional comments (22)
bindings/src/types/tari-indexer-client/GetUtxoUpdatesRequest.ts (1)
9-9: Change correctly addresses the past review comment.Making
unspent_onlyoptional aligns with the backend's#[serde(default)]attribute and allows consumers to omit the field when the default behavior is desired. Since this is a generated file (line 1), the underlying Rust type definition must have been updated accordingly.applications/tari_walletd/src/config.rs (2)
62-63: LGTM: Field renaming is consistent with the REST API migration.The field renaming from
indexer_json_rpc_urltoindexer_api_urland the doc comment update accurately reflect the migration from JSON-RPC to REST API.
99-101: Approve default indexer_api_url root base pathThe default
indexer_api_urlofhttp://127.0.0.1:18300is correct since the indexer’s REST API routes (e.g./identity,/network/stats) are mounted at the root path.clients/javascript/wallet_daemon_client/src/transports/fetch.ts (2)
32-33: LGTM!The formatting adjustment to the timeout abort call has no functional impact.
45-49: Improved error diagnostics for HTTP failures.The explicit handling of non-2xx responses before JSON parsing is a solid defensive improvement. Including the response body in the error message will significantly aid debugging when the server returns HTML error pages or detailed error responses.
bindings/src/types/tari-indexer-client/ListTemplatesRequest.ts (1)
3-3: Verify null handling forlimitfield.The
limitproperty now acceptsnull, which is a breaking change—ensure every consumer ofListTemplatesRequesthandles thenullcase appropriately and clarify whethernullindicates “no limit,” a server default, or should trigger validation errors.applications/tari_swarm_daemon/src/process_definitions/indexer.rs (2)
25-34: Past review concern addressed.The variable naming has been corrected from
json_rpc_listener_addresstoapi_listener_address, and the port retrieval now uses"api"instead of"jrpc". These changes align with the REST API migration and resolve the concerns raised in the previous review.
57-60: Approve CLI argument updates
CLI argument names (indexer.api_listen_addressandindexer.web_ui_public_api_url) align with the fields intari_indexer/src/config.rsand corresponding CLI overrides.applications/tari_indexer/web_ui/src/utils/json_rpc.tsx (3)
61-77: LGTM!The migration from
/json_rpc_addressto/rest_api_addressis correct, and the fallback toDEFAULT_API_ADDRESSprovides appropriate error handling.
79-99: LGTM!The singleton pattern with lazy initialization and promise caching correctly prevents duplicate client instances and race conditions during concurrent initialization.
116-119: LGTM!The
getNonFungiblesimplementation is now complete and correctly delegates to the REST client. The previous stub issue flagged in past reviews has been resolved.clients/javascript/indexer_client/src/index.ts (4)
1-38: LGTM!The imports are well-organized, and the re-exports (
transports, helper conversion functions) provide a cohesive public API surface.
40-57: LGTM!The class encapsulation is correct, and the static factory methods (
new,usingFetchTransport) provide an ergonomic API. ThegetTransportgetter allows access to the underlying transport when needed.
59-86: LGTM!The API methods correctly delegate to the underlying transport with appropriate endpoints and parameters.
87-117: LGTM!All path parameters are now properly URL-encoded with
encodeURIComponent(lines 88, 104, 112), which prevents URL injection and correctly handles special characters. The past review concern has been fully addressed.applications/tari_indexer/src/lib.rs (2)
109-109: LGTM: GraphQL wiring updated correctly.The change to pass
services.substate_manageraligns with the bootstrap updates that now expose substate_manager as a public field in Services.
122-131: LGTM: Past issue resolved correctly.The code now correctly uses
listen_address(the actual bound address returned byServer::spawn) instead oflisten_addr(the requested address). This ensures the Web UI points to the correct API URL even when the server binds to a different address (e.g., port 0 or fallback).Note: This addresses the past review comment about the bug in the default Web UI public API URL.
clients/tari_indexer_client/src/protobuf_stream.rs (1)
39-46: Partial length-delimiter handling now correct.The previous review flagged treating an incomplete varint as a terminal error. The current implementation correctly waits for the full varint by checking that we have fewer than 10 bytes and all have MSB = 1 before continuing to buffer. The added
tmp_slice.len() < 10guard also prevents infinite buffering when 10+ bytes all have MSB = 1 (an invalid varint), allowingdecode_length_delimiterto fail appropriately.clients/tari_indexer_client/src/types.rs (4)
311-312: Consistent FixedHash usage with TypeScript annotation.The TS annotation
number[]aligns with theFixedHashtype and matches the pattern used forbinary_shainTemplateMetadata. This breaking change ensures type consistency across the API.Ensure TypeScript clients are updated to handle
current_block_hashasnumber[]instead ofstring. This can be verified by checking the regenerated TypeScript bindings and any client code that accesses this field.
315-315: Good addition of Deserialize derive for bidirectional use.Adding
DeserializetoConnectionenables this type to be deserialized in addition to being serialized, which is essential for client code that needs to parse API responses.
377-379: Good backward compatibility for optional boolean field.The
unspent_onlyfield is well-designed for backward compatibility:
#[serde(default)]providesfalsewhen the field is missing during deserialization (handles old clients sending requests)skip_serializing_if = "std::ops::Not::not"omits the field whenfalseduring serialization (keeps JSON clean)This ensures that old clients without this field will continue to work with the default behavior of including both spent and unspent UTXOs.
159-161: Verified: TS bindings regenerated and all client code updated.
2 participants
Description
feat(indexer)!: migrate to REST api + UTXO streaming
Motivation and Context
REST api allows for streaming and openapi docs
How Has This Been Tested?
Web UI and wallet daemon updated to use indexer REST api
Cucumbers
Breaking Changes
BREAKING CHANGE: JSON-RPC api removed
Summary by CodeRabbit