Skip to content

External-signer merkle finalize reports success on partial chunk storage #166

Description

@Nic-dorman

Summary

The external-signer merkle finalize path returns Ok(FileUploadResult) even when chunks remain short of quorum after all retries — the failure is only visible in the chunks_failed field, which callers can (and do) ignore. The external-signer wave-batch path returns Err(Error::PartialUpload) in the same situation, so the two paths have opposite contracts for the same failure.

Verified identical in the published ant-core 0.5.0 and 0.5.1.

Where

  • Client::merkle_upload_chunks (ant-core/src/data/client/merkle.rs) only raises Err for fatal non-quorum errors (missing proof, chunk/address count mismatch). Chunks still short of quorum after MERKLE_STORE_MAX_ATTEMPTS (the InsufficientPeers case) are returned as counts in MerkleStoreOutcome::failed / failed_addresses.
  • Client::finalize_upload_merkle_with_progress (ant-core/src/data/client/file.rs) maps that straight into Ok(FileUploadResult { chunks_stored, chunks_failed, .. }) with chunks_failed > 0.
  • Contrast: Client::finalize_upload_with_progress (wave-batch) returns Err(Error::PartialUpload { stored, failed, .. }) if any chunk failed.

The doc comment on MerkleStoreOutcome::failed_addresses acknowledges the gap ("Used by the CLI path to build PartialUpload; the external-signer path only reads the counts"), and the comment above the outcome.fatal re-raise claims the external-signer path is "all-or-nothing", which the code doesn't deliver for quorum shortfalls.

Impact

Merkle is auto-selected for uploads of ≥64 chunks, so this affects exactly the large paid uploads where partial storage is most likely and most expensive:

  • The desktop app calls finalize_upload_merkle_with_progress, gets Ok, marks the upload complete, persists the DataMap, and shows a shareable address — for a file that cannot be fully downloaded. The user has paid on-chain.
  • The mobile FFI wrapper converts FileUploadResult to its result type and drops chunks_failed entirely, so iOS/Android external-signer integrations report success on partial storage the same way.
  • The only trace is the log line External-signer merkle upload finalized: N chunks stored, M failed.

Proposed fix

When outcome.failed > 0 in the external-signer merkle finalize, return Err(Error::PartialUpload) mirroring the wave path:

  • stored = already_stored_addresses + outcome.stored_addresses, stored_count = outcome.stored
  • failed = outcome.failed_addresses, failed_count = outcome.failed
  • total_chunks as prepared; spend unknown to the library on this path (external signer pays out-of-band)
  • update the # Errors docs and the two comments above so the contract is stated

The plumbing already exists — failed_addresses carries (address, last error) pairs.

Relationship to #140

This makes the failure visible; #140 is about making it recoverable (post-payment finalize is not retryable, so a PartialUpload after payment currently strands the payment). Fixing this one first is still strictly better than silent success: merkle proofs are reusable, so #140's resolution can later turn the surfaced PartialUpload into a free re-store instead of a re-payment. The wave path already has exactly this visibility/retryability split today.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions