fix: transaction builder remove signatures+indexer fixes

[sdbondi]

Description

fix: transaction builder remove signatures
fix(indexer): return HTTP bad request error if validators return RPC bad request
fix(indexer): bug in get transaction receipts always returning internal error

Motivation and Context

Previously, it was possible to partially sign a transaction and then make changes to the transaction (invalidating the signature(s)) using the builder. This would result in a panic.

This PR removes the internal signature collection from the builder, and changes the signing functions to return an Unsealed transaction which already has an API for adding futher signatures without allowing changes to the transaction.
Only code that previously used the builder incorrectly (i.e. would panic) will now fail to compile.

How Has This Been Tested?

Manually

What process can a PR reviewer use to test or verify this change?

Breaking Changes

• None
• Requires data directory to be deleted
• Other - Please specify

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved error handling for transaction submission failures, providing more detailed error information including transaction status and validation committee feedback.

• Refactor

  • Standardized error reporting across transaction and network interfaces for consistency and clarity.
  • Restructured transaction signing workflow for better separation of concerns and clearer API semantics.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Replaces generic RPC error generics with concrete ValidatorNodeRpcClientError, introduces ResponseErrorStatus and TransactionStatusResponseError, refactors transaction builder/signing to separate finish() and seal(), and updates numerous call sites, trait bounds, and error mappings across indexer, wallet SDK, validator RPC, and transaction crates.

Changes

Cohort / File(s)	Summary
Network client & indexer error flow applications/tari_indexer/src/network_client.rs, applications/tari_indexer/src/rest_api/handlers/transactions.rs	Replaced generic error type with ValidatorNodeRpcClientError; changed try_single/try_with_committee signatures and return collections (IndexMap); propagate concrete errors to handler which now inspects RpcStatus and maps to ResponseErrorStatus-based responses.
Indexer storage & manager applications/tari_indexer/src/storage_sqlite/reader.rs, applications/tari_indexer/src/transaction_manager/mod.rs	Use serialized receipt address hex for DB queries; make query optional and return explicit NotFound; removed IsNotFoundError bound from TransactionManager impl.
Indexer process config applications/tari_swarm_daemon/src/process_definitions/indexer.rs	Added environment variable API_DEBUG=1 to the indexer process command.
Validator node RPC API & error crates/validator_node_rpc/src/client.rs, crates/validator_node_rpc/src/error.rs	Removed associated Error type from trait; RPC methods now return ValidatorNodeRpcClientError directly; added status() helper to extract RpcStatus from error variants.
Common response status types crates/common_types/src/lib.rs, crates/common_types/src/response_status.rs	Added public response_status module with ResponseErrorStatus enum and TransactionStatusResponseError trait.
Wallet SDK / network trait changes crates/wallet/sdk/src/network.rs, crates/wallet/sdk/src/apis/transaction.rs	Replaced StatusResponseError/WalletQueryErrorStatus usages with TransactionStatusResponseError/ResponseErrorStatus; updated trait bounds and TransactionApi error variants; map IndexedValueError via explicit map_err.
Wallet SDK services & tests crates/wallet/sdk_services/src/*, crates/wallet/sdk/tests/support/harness.rs	Updated impl bounds and impls to use TransactionStatusResponseError and ResponseErrorStatus; adjusted test harness PanicError and added get_error_message().
Wallet handlers signing & transaction flows applications/tari_walletd/src/handlers/accounts.rs, applications/tari_walletd/src/handlers/nfts.rs, applications/tari_walletd/src/handlers/transaction.rs, applications/tari_walletd/src/handlers/validator.rs	Reworked signing flow to explicit two-step pattern: call with_authorized_seal_signer() where needed, call finish() earlier, then perform explicit signing (account owner then fee payer); replaced .map()-based in-builder signing.
Transaction builder & signing API crates/transaction/src/builder/mod.rs, crates/transaction/src/transaction.rs, crates/transaction/src/v1/signature.rs, crates/transaction/src/v1/unsealed.rs	Major builder refactor: removed in-builder signatures field, added finish(), seal(), add_signer()/add_signature() returning UnsealedTransactionV1, applied fee-instruction merging helpers, added TransactionSignature::sign() dispatcher, adjusted IntoSigned/unsealed behavior.
Engine & template tests, tooling, benches crates/engine/tests/*, crates/template_test_tooling/src/template_test.rs, utilities/tariswap_test_bench/src/tariswap.rs	Updated tests and tooling to use finish() + seal() two-step signing and added with_authorized_seal_signer() where appropriate; replaced build_and_seal usage with explicit finish/seal flow.

Sequence Diagram(s)

sequenceDiagram
    participant App as App / Test
    participant Builder as TransactionBuilder
    participant Unsigned as UnsignedTransaction
    participant Unsealed as UnsealedTransactionV1
    participant Signed as SignedTransaction

    rect rgb(200,240,230)
    Note over Builder,Signed: Old flow (single-step)
    App->>Builder: build_and_seal(secret)
    Builder->>Signed: finalize + seal in one step
    end

    rect rgb(220,240,200)
    Note over Builder,Signed: New flow (two-step)
    App->>Builder: finish()
    Builder->>Unsealed: produce UnsealedTransactionV1
    App->>Unsealed: seal(secret)
    Unsealed->>Signed: attach seal signature
    end

Loading

sequenceDiagram
    participant Handler as API Handler
    participant Net as NetworkClient
    participant Validator as ValidatorNodeRPC
    participant Mapper as ErrorMapper

    rect rgb(240,220,220)
    Note over Handler,Validator: Old error path
    Handler->>Net: try_single_with_committee(...)
    Net->>Validator: submit_transaction()
    Validator-->>Net: Result<T, E (generic)>
    Net-->>Handler: AllValidatorsFailed { last_error: Option<String> }
    end

    rect rgb(220,230,240)
    Note over Handler,Validator: New error path
    Handler->>Net: try_single_with_committee(...)
    Net->>Validator: submit_transaction()
    Validator-->>Net: Result<T, ValidatorNodeRpcClientError>
    Net-->>Handler: AllValidatorsFailed { last_error: Option<ValidatorNodeRpcClientError> }
    Handler->>Mapper: inspect last_error.status()
    Mapper-->>Handler: ResponseErrorStatus (NotFound/Rejected/Internal)
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Areas needing extra focus:
  • crates/transaction/src/builder/mod.rs — signature lifecycle, fee merging, and all codepaths that previously relied on in-builder signatures.
  • applications/tari_walletd/src/handlers/* and utilities/tariswap_test_bench — ensure signing order and authorization flags remain correct after converting closure-based signing to explicit steps.
  • applications/tari_indexer/src/network_client.rs and handler mapping — verify IndexMap change and extraction of RpcStatus from ValidatorNodeRpcClientError are correctly handled.
  • Cross-crate trait bound updates (StatusResponseError → TransactionStatusResponseError) — ensure every implementation maps variants and messages correctly.

Poem

🐇
I hop through diffs with nimble paws,
Errors now speak with clearer laws,
Builders finish, then seals apply,
Statuses named so bugs wave bye—
A little hop; the code breathes more.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fea30f and 140c169.

📒 Files selected for processing (28)

• applications/tari_indexer/src/network_client.rs (6 hunks)
• applications/tari_indexer/src/rest_api/handlers/transactions.rs (2 hunks)
• applications/tari_indexer/src/storage_sqlite/reader.rs (1 hunks)
• applications/tari_indexer/src/transaction_manager/mod.rs (2 hunks)
• applications/tari_swarm_daemon/src/process_definitions/indexer.rs (1 hunks)
• applications/tari_walletd/src/handlers/accounts.rs (2 hunks)
• applications/tari_walletd/src/handlers/nfts.rs (1 hunks)
• applications/tari_walletd/src/handlers/transaction.rs (5 hunks)
• applications/tari_walletd/src/handlers/validator.rs (1 hunks)
• crates/common_types/src/lib.rs (1 hunks)
• crates/common_types/src/response_status.rs (1 hunks)
• crates/engine/tests/account.rs (1 hunks)
• crates/engine/tests/stealth.rs (10 hunks)
• crates/template_test_tooling/src/template_test.rs (1 hunks)
• crates/transaction/src/builder/mod.rs (5 hunks)
• crates/transaction/src/transaction.rs (1 hunks)
• crates/transaction/src/v1/signature.rs (2 hunks)
• crates/transaction/src/v1/unsealed.rs (1 hunks)
• crates/validator_node_rpc/src/client.rs (2 hunks)
• crates/validator_node_rpc/src/error.rs (1 hunks)
• crates/wallet/sdk/src/apis/transaction.rs (8 hunks)
• crates/wallet/sdk/src/network.rs (3 hunks)
• crates/wallet/sdk/tests/support/harness.rs (4 hunks)
• crates/wallet/sdk_services/src/account_recovery/service.rs (2 hunks)
• crates/wallet/sdk_services/src/indexer_rest_api.rs (2 hunks)
• crates/wallet/sdk_services/src/transaction_service/service.rs (3 hunks)
• crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs (3 hunks)
• utilities/tariswap_test_bench/src/tariswap.rs (2 hunks)

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}