tokens: transactional core is untested and the package carries dead declarations and an unused generated mock

[Effi-S]

Fixes #2190

Summary

The transactional core of token/services/tokens has no direct unit-test coverage, despite #1487 (closed) having claimed unit-test coverage for this package. The package also carries dead exported declarations and a correspondingly unused generated mock.

Where

• Untested before this campaign: AppendValid, CacheRequest, GetCachedTokenRequest, getActions, extractActions, Parse, deleteTokens, PruneInvalidUnspentTokens, SetSpendableFlag, ContinueTransaction, Commit — the pre-existing storage_test.go (207 lines) covers only Notify/event plumbing.
• Dead declarations, confirmed to have zero references anywhere in the repo outside their own declaration:
  • type Transaction interface (tokens.go:40)
  • type GetTMSProviderFunc = func() *token.ManagementServiceProvider (tokens.go:34)
  • type UnspendableTokensIterator = driver.UnsupportedTokensIterator (tokens.go:37)
• Unused generated mock: token/services/tokens/mock/transaction.go (353 lines) exists solely to implement the dead Transaction interface above and is otherwise unreferenced. The package's mock/ directory totals 8,322 lines across 16 counterfeiter-generated files.

Impact

No regression signal exists today for the transaction-application logic that turns a validated request into local store mutations — the exact logic this campaign's other issues found defects in by writing tests for it for the first time. The dead declarations and mock add maintenance surface (they must still be kept compiling and regenerated) with no corresponding value.

Reproduction

This campaign added storage_delete_test.go, storage_notify_test.go, tokens_appendvalid_test.go, tokens_cache_test.go, tokens_parse_test.go, manager_test.go, storage_quantity_test.go, and typed_fuzz_test.go, exercising every one of the previously-untested functions above via the package's existing counterfeiter mocks and the tokens.NewDBStorage/tokens.NewTransaction/cache-seeding injection pattern already established by storage_test.go. All are passing, confirming the pattern is viable for the rest of the package's untested surface.

grep -n "tokens\.Transaction\b\|tokens\.GetTMSProviderFunc\|tokens\.UnspendableTokensIterator" -r . --include="*.go"
# no matches outside the declarations themselves

Severity

Low — a coverage and cleanup gap, not a runtime defect.

[AkramBitar]

@Effi-S

Could you please check if you addressed these requirements as well (based on my understanding to the PR description):

1. Dead code not removed.
   The PR title and issue both name three dead exported declarations and an unused generated mock, but the diff removes nothing:

type Transaction interface (tokens.go:40)
type GetTMSProviderFunc (tokens.go:34)
type UnspendableTokensIterator (tokens.go:37)
token/services/tokens/mock/transaction.go (353 lines, unreferenced)
Could you remove these as part of this PR?

2. Coverage is partial.
   The issue lists 11 untested functions but only 3 are covered here (AppendValid, GetCachedTokenRequest, SetSpendableFlag). Still missing:

CacheRequest
getActions / extractActions
Parse
deleteTokens
PruneInvalidUnspentTokens
ContinueTransaction
Commit

Regards,
Akram

[AkramBitar]

Please have a look on my review comments

[Effi-S]

@adecaro Are you OK with this removal?

type Transaction interface (tokens.go:40)
type GetTMSProviderFunc (tokens.go:34)
type UnspendableTokensIterator (tokens.go:37)
token/services/tokens/mock/transaction.go

I couldn't find them referenced anywhere.