hashgraph / hashgraph/guardian
MintFT: the idempotency checkpoint is captured unawaited and can record the mint's own timestamp, enabling a double mint on retry
- Dominant language
- TypeScript
- Stars
- 146
- Forks
- 186
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 126
Description
### Summary
The mirror-node checkpoint that makes a fungible mint idempotent is captured in a task that is **never awaited**, so it can be written with the timestamp of the mint it was meant to precede. When that happens, a retry after a timeout mints the same amount a second time on chain.
The transfer half of the same file has the identical shape, and both halves write the **same** `startTransaction` field, which gives a second route to the same outcome that does not need a race at all.
Reporting rather than opening a PR — the ordering is easy to fix, but the right trade-off between an extra mirror-node round trip and a schema addition seems like your call.
### 1. The checkpoint is not ordered before the mint
`policy-service/src/policy-engine/mint/types/mint-ft.ts`, in `mintTokens` (~line 196 on `develop`):
```ts
workers.addRetryableTask(
{
type: WorkerTaskType.GET_TRANSACTIONS,
data: { accountId: this._token.treasuryId, limit: 1, order: 'desc', transactiontype: 'TOKENMINT', ... },
},
{ priority: 1, attempts: 10, ... }
).then(async startTransactions => {
try {
this._mintRequest.startTransaction = startTransactions[0]?.consensus_timestamp;
await this._db.saveMintRequest(this._mintRequest);
} catch (error) { this.error(error, options.userId); }
}).catch(error => this.error(error, options.userId));
```
No `await`. Execution falls straight through to the mint (~line 234):
```ts
await workers.addRetryableTask(
{ type: WorkerTaskType.MINT_FT, ... },
{ priority: 10, attempts: 0, ... }
);
```
Nothing sequences the two. They also sit in different priority bands (`1` vs `10`), which routes them to different worker pools rather than ordering them, and the checkpoint carries `attempts: 10`, so it can retry well past the mint. If it resolves after our own `MINT_FT` is visible on the mirror node, `startTransaction` is set to **our own mint's** consensus timestamp.
### 2. Why that value is the idempotency fence
`resolvePendingTransactions` decides whether a `PENDING` mint already happened using exactly that value:
```ts
timestamp: this._mintRequest.startTransaction
? `gt:${this._mintRequest.startTransaction}`
: null,
filter: { memo_base64: btoa(this._mintRequest.memo) },
...
mintTransaction.mintStatus =
mintTransactions.length > 0 ? MintTransactionStatus.SUCCESS : MintTransactionStatus.NEW;
```
`gt:` is strictly greater than, so a checkpoint equal to the mint's own timestamp **excludes the very transaction it is looking for**, finds nothing, and marks the transaction `NEW`.
### 3. Failure scenario
1. `MINT_FT` times out. The catch is deliberate about this:
```ts
} catch (error) {
if (!error?.isTimeoutError) {
transaction.error = PolicyUtils.getErrorMessage(error);
transaction.mintStatus = MintTransactionStatus.ERROR;
}
throw error;
}
```
A timeout leaves the status `PENDING` precisely because the mint may have succeeded on chain.
2. The unawaited checkpoint resolves late and records our own mint's timestamp.
3. The request is retried. `resolvePendingTransactions` runs the `gt:` lookup, misses the successful mint, sets `NEW`, and the mint is re-issued.
4. The full amount is minted a second time into the treasury and transferred on.
The window is not exotic: a timeout is the only case where the retry path exists at all, and a slow mirror-node query is the same condition that makes the checkpoint land late.
### 4. Both halves share one field — a second, race-free route
`transferTokens` has the identical unawaited capture (~line 295) and its own `gt:` fence, and both halves assign the **same** field:
```ts
this._mintRequest.startTransaction = startTransactions[0]?.consensus_timestamp;
```
`startTransaction` is a single optional column on `MintRequest` (`common/src/entity/mint-request.ts`).
Mint runs before transfer, so the transfer's checkpoint — a later `CRYPTOTRANSFER` timestamp — overwrites the mint's. Any subsequent mint re-resolution then fences on a timestamp *after* the mint occurred, misses it, and re-mints. That needs no race: it follows deterministically once a transfer checkpoint has landed.
### Proposed fix
Two parts, and the second may be the more interesting one:
1. **Await each checkpoint before its own side effect**, in both halves, so `startTransaction` is by construction earlier than the transaction it fences. This is the minimal change and closes the reported race.
2. **Give the transfer its own checkpoint field**, so the two cannot clobber each other. Additive on the entity; existing rows read as `undefined`, and the fence already handles that by falling back to `timestamp: null` plus the memo filter — which is the safe direction, since it *would* find the transaction.
A checkpoint written after the fact is worse than none: with `startTransaction` unset the query falls back to the memo filter alone, which would have found the mint.
### Trade-off worth your judgement
(1) adds a mirror-node round trip before every mint and every transfer. If that latency is unacceptable, the timestamp could instead be captured once at request creation — earlier than both side effects by construction and paid once — but it has to be ordered before the side effect either way.
I have not opened a PR, since the choice between awaiting per operation, hoisting the capture to request creation, and whether to add the second column is a design decision rather than a mechanical fix. Happy to submit whichever shape you prefer.
Contributor guide
Research direction
Read policy-service/src/policy-engine/mint/types/mint-ft.ts at mintTokens and transferTokens, then trace resolvePendingTransactions and common/src/entity/mint-request.ts. Check how the unawaited checkpoint tasks assign startTransaction and how the gt: lookup handles retries. Done means the chosen ordering and checkpoint design prevent a timed-out mint from being issued twice without the two operation types clobbering one another.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend, databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100