Skip to content

Commit 520c80f

Browse files
authored
refactor(recovery): return RecoveryClaim from ClaimPendingTransactions (#1715)
Signed-off-by: Evan <evanyan@sign.global>
1 parent 7270905 commit 520c80f

12 files changed

Lines changed: 216 additions & 219 deletions

File tree

token/services/auditor/mock/audit_transaction_store.go

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

token/services/network/fabric/network.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,6 @@ type transactionDB interface {
618618
GetTokenRequest(ctx context.Context, txID string) ([]byte, error)
619619
SetStatus(ctx context.Context, txID string, status storage.TxStatus, message string) error
620620
AcquireRecoveryLeadership(ctx context.Context, lockID int64) (recovery.Leadership, bool, error)
621-
ClaimPendingTransactions(ctx context.Context, olderThan time.Duration, leaseDuration time.Duration, limit int, owner string) ([]*ttxdb.TransactionRecord, error)
621+
ClaimPendingTransactions(ctx context.Context, olderThan time.Duration, leaseDuration time.Duration, limit int, owner string) ([]*ttxdb.RecoveryClaim, error)
622622
ReleaseRecoveryClaim(ctx context.Context, txID string, owner string, message string) error
623623
}

token/services/storage/auditdb/store.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ type MovementRecord = dbdriver.MovementRecord
103103
// in that action.
104104
type TransactionRecord = dbdriver.TransactionRecord
105105

106+
// RecoveryClaim is the minimal projection of a pending transaction row
107+
// returned by ClaimPendingTransactions for recovery processing.
108+
type RecoveryClaim = dbdriver.RecoveryClaim
109+
106110
// QueryTransactionsParams defines the parameters for querying movements
107111
type QueryTransactionsParams = dbdriver.QueryTransactionsParams
108112

@@ -353,7 +357,9 @@ func (d *StoreService) AcquireRecoveryLeadership(ctx context.Context, lockID int
353357
}
354358

355359
// ClaimPendingTransactions returns a claimed batch of Pending transactions older than the given duration.
356-
func (d *StoreService) ClaimPendingTransactions(ctx context.Context, olderThan time.Duration, leaseDuration time.Duration, limit int, owner string) ([]*TransactionRecord, error) {
360+
// Each returned RecoveryClaim carries the TxID and StoredAt timestamp the recovery loop needs;
361+
// the rest of the row is intentionally not projected from SQL.
362+
func (d *StoreService) ClaimPendingTransactions(ctx context.Context, olderThan time.Duration, leaseDuration time.Duration, limit int, owner string) ([]*RecoveryClaim, error) {
357363
storedBefore := time.Now().UTC().Add(-olderThan)
358364
logger.DebugfContext(ctx, "claiming pending transactions stored before %s (older than %s), lease duration [%s], limit [%d], owner [%s]",
359365
storedBefore, olderThan, leaseDuration, limit, owner)

token/services/storage/db/driver/audit.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ type AuditTransactionStore interface {
5555

5656
// ClaimPendingTransactions atomically claims a batch of Pending transactions for recovery processing.
5757
// Transactions whose recovery lease expired are eligible again.
58-
ClaimPendingTransactions(ctx context.Context, params RecoveryClaimParams) ([]*TransactionRecord, error)
58+
// Returns the minimal projection (TxID + StoredAt) needed by the recovery loop;
59+
// callers do not need the full TransactionRecord.
60+
ClaimPendingTransactions(ctx context.Context, params RecoveryClaimParams) ([]*RecoveryClaim, error)
5961

6062
// ReleaseRecoveryClaim clears the recovery claim metadata for the given transaction if owned by owner.
6163
// The message parameter is stored for audit/debugging purposes.

token/services/storage/db/driver/ttx.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ type TransactionStore interface {
9595

9696
// ClaimPendingTransactions atomically claims a batch of Pending transactions for recovery processing.
9797
// Transactions whose recovery lease expired are eligible again.
98-
ClaimPendingTransactions(ctx context.Context, params RecoveryClaimParams) ([]*TransactionRecord, error)
98+
// Returns the minimal projection (TxID + StoredAt) needed by the recovery loop;
99+
// callers do not need the full TransactionRecord.
100+
ClaimPendingTransactions(ctx context.Context, params RecoveryClaimParams) ([]*RecoveryClaim, error)
99101

100102
// ReleaseRecoveryClaim clears the recovery claim metadata for the given transaction if owned by owner.
101103
// The message parameter is stored for audit/debugging purposes.
@@ -124,6 +126,20 @@ type RecoveryClaimParams struct {
124126
Owner string
125127
}
126128

129+
// RecoveryClaim is the minimal projection of a pending transaction row
130+
// returned by ClaimPendingTransactions. The recovery loop only needs the
131+
// TxID to act on and the StoredAt timestamp to decide grace-period
132+
// promotions; the rest of TransactionRecord (action type, amounts,
133+
// metadata, ...) was always discarded by the caller, so the SQL layer
134+
// stops projecting it.
135+
type RecoveryClaim struct {
136+
// TxID is the transaction ID claimed for recovery.
137+
TxID string
138+
// StoredAt is the storage timestamp of the underlying row (UTC), used
139+
// by the recovery loop to compute row age for grace-period decisions.
140+
StoredAt time.Time
141+
}
142+
127143
// TransactionRecordReference contains the primary key fields of a transaction request record.
128144
type TransactionRecordReference struct {
129145
// TxID is the unique identifier of the transaction request.

token/services/storage/db/sql/common/transactions.go

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -327,23 +327,19 @@ func (db *TransactionStore) AcquireRecoveryLeadership(ctx context.Context, lockI
327327

328328
// ClaimPendingTransactions returns a claimed batch of Pending transactions.
329329
// The default SQL implementation is permissive and does not persist recovery claims.
330-
func (db *TransactionStore) ClaimPendingTransactions(ctx context.Context, params dbdriver.RecoveryClaimParams) ([]*dbdriver.TransactionRecord, error) {
331-
transactionsTable, requestsTable := q.Table(db.table.Transactions), q.Table(db.table.Requests)
330+
// tx_id and stored_at are projected directly from the requests table — the
331+
// transactions table is no longer joined since it carries no information
332+
// the recovery loop needs and adding it would re-introduce a fan-out by
333+
// movement/output that the caller would have to dedupe by tx_id.
334+
func (db *TransactionStore) ClaimPendingTransactions(ctx context.Context, params dbdriver.RecoveryClaimParams) ([]*dbdriver.RecoveryClaim, error) {
332335
query, args := q.Select().
333-
Fields(
334-
transactionsTable.Field("tx_id"), common3.FieldName("action_type"), common3.FieldName("sender_eid"),
335-
common3.FieldName("recipient_eid"), common3.FieldName("token_type"), common3.FieldName("amount"),
336-
requestsTable.Field("status"), requestsTable.Field("application_metadata"),
337-
requestsTable.Field("public_metadata"), transactionsTable.Field("stored_at"),
338-
).
339-
From(transactionsTable.Join(requestsTable,
340-
cond.Cmp(transactionsTable.Field("tx_id"), "=", requestsTable.Field("tx_id"))),
341-
).
336+
FieldsByName("tx_id", "stored_at").
337+
From(q.Table(db.table.Requests)).
342338
Where(cond.And(
343339
cond.Eq("status", dbdriver.Pending),
344-
cond.Lt(common3.FieldName(db.table.Transactions+".stored_at"), params.OlderThan),
340+
cond.Lt("stored_at", params.OlderThan),
345341
)).
346-
OrderBy(q.Asc(transactionsTable.Field("stored_at"))).
342+
OrderBy(q.Asc(common3.FieldName("stored_at"))).
347343
Limit(params.Limit).
348344
Format(db.ci)
349345

@@ -353,19 +349,8 @@ func (db *TransactionStore) ClaimPendingTransactions(ctx context.Context, params
353349
return nil, err
354350
}
355351

356-
results := common.NewIterator(rows, func(r *dbdriver.TransactionRecord) error {
357-
var amount BigInt
358-
var appMeta []byte
359-
var pubMeta []byte
360-
if err := rows.Scan(&r.TxID, &r.ActionType, &r.SenderEID, &r.RecipientEID, &r.TokenType, &amount, &r.Status, &appMeta, &pubMeta, &r.Timestamp); err != nil {
361-
return err
362-
}
363-
r.Amount = amount.Int
364-
365-
return errors2.Join(
366-
unmarshal(appMeta, &r.ApplicationMetadata),
367-
unmarshal(pubMeta, &r.PublicMetadata),
368-
)
352+
results := common.NewIterator(rows, func(r *dbdriver.RecoveryClaim) error {
353+
return rows.Scan(&r.TxID, &r.StoredAt)
369354
})
370355

371356
return iterators.ReadAllPointers(results)

token/services/storage/db/sql/postgres/recovery_claim_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package postgres
88

99
import (
1010
"context"
11+
"fmt"
1112
"math/big"
1213
"testing"
1314
"time"
@@ -49,8 +50,10 @@ func TestClaimPendingTransactions_Atomic(t *testing.T) {
4950
oldTime := now.Add(-10 * time.Minute)
5051

5152
// Add 5 pending transactions
53+
txIDs := make([]string, 0, 5)
5254
for i := range 5 {
5355
txID := "tx" + string(rune('1'+i))
56+
txIDs = append(txIDs, txID)
5457
err = aw.AddTokenRequest(ctx, txID, []byte("request"), nil, nil, []byte("hash"))
5558
require.NoError(t, err)
5659

@@ -68,6 +71,7 @@ func TestClaimPendingTransactions_Atomic(t *testing.T) {
6871

6972
err = aw.Commit()
7073
require.NoError(t, err)
74+
ageRequests(t, ctx, store1, oldTime, txIDs...)
7175

7276
// Both instances try to claim the same transactions
7377
params := tokensdriver.RecoveryClaimParams{
@@ -128,6 +132,7 @@ func TestClaimPendingTransactions_Lease(t *testing.T) {
128132

129133
err = aw.Commit()
130134
require.NoError(t, err)
135+
ageRequests(t, ctx, store, oldTime, txID)
131136

132137
// Claim with very short lease
133138
params := tokensdriver.RecoveryClaimParams{
@@ -196,6 +201,7 @@ func TestClaimPendingTransactions_Idempotent(t *testing.T) {
196201

197202
err = aw.Commit()
198203
require.NoError(t, err)
204+
ageRequests(t, ctx, store, oldTime, txID)
199205

200206
// Claim transaction
201207
params := tokensdriver.RecoveryClaimParams{
@@ -238,8 +244,10 @@ func TestClaimPendingTransactions_Limit(t *testing.T) {
238244
now := time.Now().UTC()
239245
oldTime := now.Add(-10 * time.Minute)
240246

247+
txIDs := make([]string, 0, 10)
241248
for i := range 10 {
242249
txID := "tx" + string(rune('0'+i))
250+
txIDs = append(txIDs, txID)
243251
err = aw.AddTokenRequest(ctx, txID, []byte("request"), nil, nil, []byte("hash"))
244252
require.NoError(t, err)
245253

@@ -257,6 +265,9 @@ func TestClaimPendingTransactions_Limit(t *testing.T) {
257265

258266
err = aw.Commit()
259267
require.NoError(t, err)
268+
for i, txID := range txIDs {
269+
ageRequests(t, ctx, store, oldTime.Add(time.Duration(i)*time.Second), txID)
270+
}
260271

261272
// Claim with limit of 3
262273
params := tokensdriver.RecoveryClaimParams{
@@ -317,6 +328,7 @@ func TestReleaseRecoveryClaim(t *testing.T) {
317328

318329
err = aw.Commit()
319330
require.NoError(t, err)
331+
ageRequests(t, ctx, store, oldTime, txID)
320332

321333
// Claim transaction
322334
params := tokensdriver.RecoveryClaimParams{
@@ -381,6 +393,7 @@ func TestReleaseRecoveryClaim_WrongOwner(t *testing.T) {
381393

382394
err = aw.Commit()
383395
require.NoError(t, err)
396+
ageRequests(t, ctx, store, oldTime, txID)
384397

385398
// Claim transaction
386399
params := tokensdriver.RecoveryClaimParams{
@@ -427,8 +440,10 @@ func TestCleanupExpiredClaims(t *testing.T) {
427440
now := time.Now().UTC()
428441
oldTime := now.Add(-10 * time.Minute)
429442

443+
txIDs := make([]string, 0, 3)
430444
for i := range 3 {
431445
txID := "tx" + string(rune('1'+i))
446+
txIDs = append(txIDs, txID)
432447
err = aw.AddTokenRequest(ctx, txID, []byte("request"), nil, nil, []byte("hash"))
433448
require.NoError(t, err)
434449

@@ -446,6 +461,7 @@ func TestCleanupExpiredClaims(t *testing.T) {
446461

447462
err = aw.Commit()
448463
require.NoError(t, err)
464+
ageRequests(t, ctx, store, oldTime, txIDs...)
449465

450466
// Claim with very short lease
451467
params := tokensdriver.RecoveryClaimParams{
@@ -473,3 +489,18 @@ func TestCleanupExpiredClaims(t *testing.T) {
473489
require.NoError(t, err)
474490
require.Len(t, claimed, 3, "Should be able to claim after cleanup")
475491
}
492+
493+
func ageRequests(t *testing.T, ctx context.Context, store *TransactionStore, storedAt time.Time, txIDs ...string) {
494+
t.Helper()
495+
496+
// #nosec G201 -- table name comes from the test-created store.
497+
query := fmt.Sprintf("UPDATE %s SET stored_at = $1 WHERE tx_id = $2", store.tables.Requests)
498+
for _, txID := range txIDs {
499+
result, err := store.writeDB.ExecContext(ctx, query, storedAt, txID)
500+
require.NoError(t, err)
501+
502+
rowsAffected, err := result.RowsAffected()
503+
require.NoError(t, err)
504+
require.EqualValues(t, 1, rowsAffected)
505+
}
506+
}

0 commit comments

Comments
 (0)