feat(action-token): reserve link tokens with transfer_pending - #579
Merged
mahdi2ba merged 3 commits intoSep 21, 2026
Merged
Conversation
Share link generation took the first N tokens of the wallet, reserved or not, while every transfer path skips reserved ones. The link was issued with no warning and redeem refused it with a 409 that landed on the recipient, who could do nothing about it. Add Token.getAvailableTokens, filtering transfer_pending and claim, and page over that when picking a bundle. The existing count check then produces the 409 at generate time, where the sender can act on it. getByOwner is untouched, so GET /tokens still shows a wallet's full holdings. Apply the same rule to the explicit ids path, which checked ownership only. This is what Transfer.transferActionToken already enforces at claim time, moved earlier. Refs Greenstand#571
Two reservation mechanisms existed and neither saw the other. A share link recorded its tokens in action_token, which only link generation consulted, while every transfer path honours token.transfer_pending. So a normal send took the token a live link had promised, and the link died at claim with a 409 the sender never saw. Reuse the flag every path already honours rather than teaching each path about action_token: - New nullable action_token_id column on token, additive so v1 and v2 ignore it. transfer_pending_id is left alone: it is read elsewhere as a transfer id, and keeping it null for link tokens means a transfer cancel can never release a link's tokens by accident. - generate() claims the tokens with a conditional update and requires the full count, so two concurrent creations cannot both take the same row. - cancel() and redeem() hand them back, redeem inside its own transaction so a link whose record cannot be read fails closed. - Expired links release lazily on any generate() or list() by anyone, so no cron job is needed. redeemActionToken now reads its tokens on the transaction's own session. A separate TokenService opens its own connection and would not see the release. Refs Greenstand#574
generate() flags the tokens first and inserts the action_token row after. If the conditional update flagged fewer rows than asked (a concurrent link or send took some between selection and update), or the insert failed, the flagged rows kept an action_token_id that no row will ever carry. Cancel, redeem and the lazy expiry all release by that id through the row, so those tokens stayed unsendable for good. Release by the id on both failure paths before rethrowing. Refs Greenstand#574 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
🎉 This PR is included in version 1.44.0-keycloak.30 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
mahdi2ba
added a commit
that referenced
this pull request
Sep 21, 2026
…elf-healing Follow-ups to #579 (#574), from its review. - generate() flags the tokens and inserts the link row in one transaction. A short reservation (a concurrent link or send got there first) or a failed insert rolls the flags back, so no token can stay flagged for a link that does not exist. The UPDATE is also scoped to the sender wallet. - Explicit token ids must belong to the sender wallet. walletB could name a token of walletC, a wallet it manages, without sender_wallet; the link then spoke for walletB and claim time refused it forever. Refuse at generate time, to the sender, with a clear message. - Housekeeping releases every token flagged for a link that is not active, in one statement (NOT EXISTS active row), instead of a per-id loop after expiry. That also heals whatever a crash between two statements left behind. cancel() updates state and releases in one transaction. - New migration turns token_action_token_id_idx into a partial index on action_token_id IS NOT NULL: nearly every row is NULL, so the index stays tiny and off the write path of ordinary token updates. - GET /action-tokens?state=expired is accepted; rows can be expired since #579. Three integration specs: an explicit token of a managed wallet without sender_wallet is refused and flags nothing; a token flagged for a link with no row is released on the next link activity; expired links list by state. Refs #574 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #577. The diff shows both commits until that merges.
Problem
Two reservation mechanisms existed and neither saw the other. A share link recorded its tokens in
action_token, which only link generation consulted, while every transfer path honourstoken.transfer_pending. So a normal send took the token a live link had promised, and the link died at claim with a 409 the sender never saw.Fix
Reuse the flag every path already honours, rather than teaching each path about
action_token.action_token_idcolumn ontoken. Additive, so v1 and v2 ignore it.transfer_pending_idis left alone: it is read elsewhere as a transfer id, and keeping it null for link tokens means a transfer cancel can never release a link's tokens by accident.generate()claims the tokens with a conditional update and requires the full count, so two concurrent creations cannot both take the same row.cancel()andredeem()hand them back, redeem inside its own transaction so a link whose record cannot be read fails closed.generate()orlist()by anyone, so no cron job is needed.One fix that was not in the issue
redeemActionTokencreatednew TokenService(), which opens its own Session outside the open transaction, so it could not see the release. It now reads on the transaction's session.Regression notes
A pending normal send reserves and releases exactly as #560 does,
transfer_pending_iduntouched.fulfillTransferpicks only free tokens, so it can no longer take a link's token. Links created before this deploy have a row but unflagged tokens, and keep redeeming; they are simply unprotected until claimed or cancelled.actiontoken-skip-reserved.spec.jsfrom #577 now asserts the new behaviour: promised tokens are flagged and carry the link's id.Testing
Six new integration specs: link and send take different tokens and the link still redeems; the last free token being promised refuses a send; cancel frees it; two concurrent links leave exactly one alive; an expired link releases on the next link activity; redeem clears the flag.
Unit 241 passing, integration 141 passing across three runs.
Refs #574