Skip to content

fix(vault): route the two ways out of a pending transfer - #194

Open
LKSNDRTMLKV wants to merge 1 commit into
refactor/repoint-core-layoutfrom
fix/transfer-reject-cancel-routes
Open

fix(vault): route the two ways out of a pending transfer#194
LKSNDRTMLKV wants to merge 1 commit into
refactor/repoint-core-layoutfrom
fix/transfer-reject-cancel-routes

Conversation

@LKSNDRTMLKV

Copy link
Copy Markdown
Member

TransferRecord::reject() and ::cancel() have existed in core since the transfer handshake landed. Nothing in the engine called either — grep across all crates returned zero, and the router carried exactly two transfer routes.

That combines badly with the chain's own guard. TransferChain::initiate_transfer refuses a new handover while any record is Initiated or Accepted. Accepted is transient (accept completes in the same call), but Initiated is not: a counterparty that never acts leaves the record pending forever, and every later transfer on that passport is refused with TransferAlreadyPending, with no route able to clear it.

Core built the escape hatch. The vault never opened it.

What this adds

  • POST /api/v1/dpp/{dppId}/transfer/reject — the incoming operator refuses. Terminal.
  • POST /api/v1/dpp/{dppId}/transfer/cancel — the outgoing operator withdraws. Terminal, and valid from one state more, since core permits a cancel after the acceptance step has run.

Both go through one shared terminate_pending_transfer, because the two differ only in which core method they call and what they are called in the audit trail. Its selection predicate deliberately mirrors initiate_transfer's own has_pending check — whatever blocks a new transfer is exactly what these clear. Legality is not re-decided in the vault: the record's own state machine refuses a reject from anything but Initiated, so this selects a candidate and lets core say no.

No registry notification is enqueued. A notification is queued when a handover completes; a transfer ending here never completed, so the registry was never told it was coming and is owed nothing. A plain chain write is the whole of the persistence.

Test

a_pending_transfer_blocks_until_rejected_or_cancelled walks the full shape rather than asserting the two routes return 200:

block → reject → clear → block again → cancel → clear → and a terminate with nothing pending is refused.

Verification — please read before merging

just check is red locally, on one test, and it is not from this change:

dpp-integrator handlers::schemas::tests::no_embedded_schema_keeps_a_description_after_stripping

Two pieces of evidence that it is pre-existing and environmental:

  1. It fails identically on this branch's base with these changes stashed out.
  2. It is a core-local artefact. The test fails on unsold-goods v2.0.0, and that schema file was added by dpp-core chore(deps): bump const-oid from 0.9.6 to 0.10.2 #186 — present in dpp-core main, absent from published 0.18.0 (nothing touched that path before the 2026-08-19 release). CI has no .cargo/config.toml, so it resolves published 0.18.0, where the schema does not exist.

So CI should be green here. I have not independently confirmed that — CI on this PR is the check. If it goes red, this needs a second look before merge.

Everything else passed: cargo nextest run --workspace --no-fail-fast863/864, the one failure being the above. fmt, clippy, the OpenAPI contract test, spec-version, outbound, grants and migrations checks all green, and just openapi-check passes with the regenerated bundles committed.

Base branch — your call

This is based on refactor/repoint-core-layout (#189), not main, so it inherits that draft's block on core 0.19.0.

The reason is mechanical: main still expects the pre-#188 core module layout, while the local .cargo patch points at a core checkout that has #188. Building against main would need a core checkout pinned to published 0.18.0, and there is no local v0.18.0 tag.

This is a bug fix and arguably should not wait on a release train. Say the word and I will rebase it onto main.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 27 complexity · 20 duplication

Metric Results
Complexity 27
Duplication 20

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@LKSNDRTMLKV

Copy link
Copy Markdown
Member Author

Correction — my verification note above is wrong about CI.

CI is red, and not for the reason the description predicts. The failure is:

error[E0432]: unresolved imports `dpp_domain::passport`, `dpp_domain::transfer`
             could not find `passport` in `dpp_domain`

CI resolves dpp-domain = "0.18.0" from the registry, which still has the pre-dissolution dpp_domain::domain::* layout. The base branch repoints every import to the new one. The entire base does not compile in CI, which is precisely why its own PR is a draft blocked on core 0.19.0.

The description says this PR "inherits that draft's block" and then predicts CI would be green two paragraphs later. Both cannot be true, and the second is the wrong one. The dpp-integrator analysis in the description is still accurate as far as it goes — that test is pre-existing and environmental — but it is not what CI is failing on.

On the base branch, reassessed. I tried rebasing onto main so this bug fix could merge independently. That is not cheap: main predates the product-group rename, has no api/openapi.bundled.json, and pins core 0.18.0, so the rebase conflicts on the bundle and would need the change re-adapted to the older vocabulary — then re-adapted back when the line merges. Basing on the repoint line was the right call after all.

Net: this PR is blocked on core 0.19.0 publishing, exactly like its parent. That is inherent to the line rather than something this change introduced, and it is the same release that unblocks #189. No code change is needed here; the verification claim in the description is what was wrong, and this comment supersedes it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant