From 0a6969e86ff63ec0a8a59d4d0f998e38d5a0b6f1 Mon Sep 17 00:00:00 2001 From: Maxence Maireaux Date: Thu, 11 Jun 2026 10:02:51 +0200 Subject: [PATCH 1/3] fix: make wallet creation and pending debits idempotent Creation paths generated a fresh UUID on every request, so retrying with the same Idempotency-Key created duplicates (and for pending debits the ledger rejected the retry, since the hold ID baked into the request body changed each time). - Derive the wallet ID and the pending-debit hold ID from the Idempotency-Key (UUIDv5) when one is provided; keep random IDs otherwise - Forward the Idempotency-Key to the ledger when creating a wallet Adds tests asserting stable IDs and key forwarding across retries. --- pkg/api/handler_wallets_create.go | 3 ++- pkg/api/handler_wallets_create_test.go | 33 +++++++++++++++++++++++++ pkg/api/handler_wallets_debit_test.go | 34 ++++++++++++++++++++++++++ pkg/manager.go | 26 ++++++++++++++++++-- 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/pkg/api/handler_wallets_create.go b/pkg/api/handler_wallets_create.go index 06d1d6b..1d9d95b 100644 --- a/pkg/api/handler_wallets_create.go +++ b/pkg/api/handler_wallets_create.go @@ -3,6 +3,7 @@ package api import ( "net/http" + "github.com/formancehq/go-libs/v5/pkg/transport/api" wallet "github.com/formancehq/wallets/pkg" "github.com/go-chi/render" ) @@ -16,7 +17,7 @@ func (m *MainHandler) createWalletHandler(w http.ResponseWriter, r *http.Request } } - wallet, err := m.manager.CreateWallet(r.Context(), data) + wallet, err := m.manager.CreateWallet(r.Context(), api.IdempotencyKeyFromRequest(r), data) if err != nil { internalError(w, r, err) return diff --git a/pkg/api/handler_wallets_create_test.go b/pkg/api/handler_wallets_create_test.go index f656c07..37f9b8c 100644 --- a/pkg/api/handler_wallets_create_test.go +++ b/pkg/api/handler_wallets_create_test.go @@ -51,3 +51,36 @@ func TestWalletsCreate(t *testing.T) { require.Equal(t, wallet.Metadata, createWalletRequest.Metadata) require.Equal(t, wallet.Name, createWalletRequest.Name) } + +func TestWalletsCreateIdempotency(t *testing.T) { + t.Parallel() + + const idempotencyKey = "create-wallet-key-1" + + var forwardedKeys []string + testEnv := newTestEnv( + WithAddMetadataToAccount(func(ctx context.Context, l, a, ik string, m map[string]string) error { + forwardedKeys = append(forwardedKeys, ik) + return nil + }), + ) + + create := func() *wallet.Wallet { + req := newRequest(t, http.MethodPost, "/wallets", wallet.CreateRequest{Name: uuid.NewString()}) + req.Header.Set("Idempotency-Key", idempotencyKey) + rec := httptest.NewRecorder() + testEnv.Router().ServeHTTP(rec, req) + require.Equal(t, http.StatusCreated, rec.Result().StatusCode) + w := &wallet.Wallet{} + readResponse(t, rec, w) + return w + } + + first := create() + second := create() + + // The Idempotency-Key is forwarded to the ledger and the derived wallet + // ID is stable across retries, so no duplicate wallet is created. + require.Equal(t, []string{idempotencyKey, idempotencyKey}, forwardedKeys) + require.Equal(t, first.ID, second.ID) +} diff --git a/pkg/api/handler_wallets_debit_test.go b/pkg/api/handler_wallets_debit_test.go index 4728941..83a7aed 100644 --- a/pkg/api/handler_wallets_debit_test.go +++ b/pkg/api/handler_wallets_debit_test.go @@ -416,3 +416,37 @@ func TestWalletsDebit(t *testing.T) { }) } } + +func TestWalletsDebitPendingIdempotency(t *testing.T) { + t.Parallel() + + const idempotencyKey = "debit-pending-key-1" + walletID := uuid.NewString() + + testEnv := newTestEnv( + WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) { + require.Equal(t, idempotencyKey, ik) + //nolint:nilnil + return nil, nil + }), + ) + + debit := func() *wallet.DebitHold { + req := newRequest(t, http.MethodPost, "/wallets/"+walletID+"/debit", wallet.DebitRequest{ + Amount: wallet.NewMonetary(big.NewInt(100), "USD"), + Pending: true, + }) + req.Header.Set("Idempotency-Key", idempotencyKey) + rec := httptest.NewRecorder() + testEnv.Router().ServeHTTP(rec, req) + require.Equal(t, http.StatusCreated, rec.Result().StatusCode) + hold := &wallet.DebitHold{} + readResponse(t, rec, hold) + return hold + } + + // Retrying a pending debit with the same Idempotency-Key yields the same + // hold ID, so the ledger request body is identical and no duplicate hold + // is created. + require.Equal(t, debit().ID, debit().ID) +} diff --git a/pkg/manager.go b/pkg/manager.go index c9566d6..8cc189a 100644 --- a/pkg/manager.go +++ b/pkg/manager.go @@ -14,9 +14,20 @@ import ( "github.com/formancehq/formance-sdk-go/v3/pkg/models/shared" "github.com/formancehq/go-libs/v5/pkg/types/metadata" + "github.com/google/uuid" "github.com/pkg/errors" ) +// idempotencyNamespace seeds deterministic resource IDs derived from an +// Idempotency-Key, so that retrying a creation request resolves to the same +// wallet/hold rather than creating a duplicate. +var idempotencyNamespace = uuid.MustParse("0b6f2d6e-4e2a-4f3a-9f0a-2b9c1d8e7a31") + +// deterministicID derives a stable UUID from an Idempotency-Key. +func deterministicID(ik string) string { + return uuid.NewSHA1(idempotencyNamespace, []byte(ik)).String() +} + type ListResponse[T any] struct { Data []T Next, Previous string @@ -126,6 +137,12 @@ func (m *Manager) Debit(ctx context.Context, ik string, debit Debit) (*DebitHold } hold = Ptr(debit.newHold()) + // Derive the hold ID from the Idempotency-Key so a retry produces an + // identical ledger request (the ledger hashes the body to enforce + // idempotency) and returns the same hold. + if ik != "" { + hold.ID = deterministicID(ik) + } holdAccount := m.chart.GetHoldAccount(hold.ID) metadata = map[string]map[string]string{ holdAccount: hold.LedgerMetadata(m.chart), @@ -418,14 +435,19 @@ func (m *Manager) ListTransactions(ctx context.Context, query ListQuery[ListTran }), nil } -func (m *Manager) CreateWallet(ctx context.Context, data *CreateRequest) (*Wallet, error) { +func (m *Manager) CreateWallet(ctx context.Context, ik string, data *CreateRequest) (*Wallet, error) { wallet := NewWallet(data.Name, m.ledgerName, data.Metadata) + // Derive the wallet ID from the Idempotency-Key so a retry targets the + // same account instead of creating a duplicate wallet. + if ik != "" { + wallet.ID = deterministicID(ik) + } if err := m.client.AddMetadataToAccount( ctx, m.ledgerName, m.chart.GetMainBalanceAccount(wallet.ID), - "", + ik, wallet.LedgerMetadata(), ); err != nil { return nil, errors.Wrap(err, "adding metadata to account") From 71db09958d27170bf7589173c2041720c196f09f Mon Sep 17 00:00:00 2001 From: Maxence Maireaux Date: Thu, 11 Jun 2026 12:45:30 +0200 Subject: [PATCH 2/3] test: model true idempotent replays and document pending-debit limits Address review feedback: - Wallet-create idempotency test now replays a fixed payload (a regenerated name would be a different body for the same key, i.e. a conflict, not a replay) and asserts the targeted account is stable across retries. - Strengthen the pending-debit idempotency test to assert a byte-identical ledger body across retries for a stable source set. - Document that pending-debit idempotency only holds for an explicit, non-expiring source set: the wildcard / expiry-crossing case is resolved against live state with time.Now() and may still differ on retry. --- pkg/api/handler_wallets_create_test.go | 19 +++++++++++++++---- pkg/api/handler_wallets_debit_test.go | 11 ++++++++--- pkg/manager.go | 10 ++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/pkg/api/handler_wallets_create_test.go b/pkg/api/handler_wallets_create_test.go index 37f9b8c..4a3175c 100644 --- a/pkg/api/handler_wallets_create_test.go +++ b/pkg/api/handler_wallets_create_test.go @@ -57,16 +57,25 @@ func TestWalletsCreateIdempotency(t *testing.T) { const idempotencyKey = "create-wallet-key-1" - var forwardedKeys []string + // A real idempotent retry replays the *same* request body, so the payload + // is fixed across both calls (a regenerated name would be a different body + // with the same key, which the ledger treats as a conflict, not a replay). + request := wallet.CreateRequest{Name: "savings-account"} + + var ( + forwardedKeys []string + targetedAccounts []string + ) testEnv := newTestEnv( WithAddMetadataToAccount(func(ctx context.Context, l, a, ik string, m map[string]string) error { forwardedKeys = append(forwardedKeys, ik) + targetedAccounts = append(targetedAccounts, a) return nil }), ) create := func() *wallet.Wallet { - req := newRequest(t, http.MethodPost, "/wallets", wallet.CreateRequest{Name: uuid.NewString()}) + req := newRequest(t, http.MethodPost, "/wallets", request) req.Header.Set("Idempotency-Key", idempotencyKey) rec := httptest.NewRecorder() testEnv.Router().ServeHTTP(rec, req) @@ -79,8 +88,10 @@ func TestWalletsCreateIdempotency(t *testing.T) { first := create() second := create() - // The Idempotency-Key is forwarded to the ledger and the derived wallet - // ID is stable across retries, so no duplicate wallet is created. + // The Idempotency-Key is forwarded to the ledger and the derived wallet ID + // (hence the targeted account) is stable across retries, so the retry hits + // the same account instead of creating a duplicate wallet. require.Equal(t, []string{idempotencyKey, idempotencyKey}, forwardedKeys) require.Equal(t, first.ID, second.ID) + require.Equal(t, targetedAccounts[0], targetedAccounts[1]) } diff --git a/pkg/api/handler_wallets_debit_test.go b/pkg/api/handler_wallets_debit_test.go index 83a7aed..20f5f17 100644 --- a/pkg/api/handler_wallets_debit_test.go +++ b/pkg/api/handler_wallets_debit_test.go @@ -423,9 +423,11 @@ func TestWalletsDebitPendingIdempotency(t *testing.T) { const idempotencyKey = "debit-pending-key-1" walletID := uuid.NewString() + var bodies []wallet.PostTransaction testEnv := newTestEnv( WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) { require.Equal(t, idempotencyKey, ik) + bodies = append(bodies, p) //nolint:nilnil return nil, nil }), @@ -445,8 +447,11 @@ func TestWalletsDebitPendingIdempotency(t *testing.T) { return hold } - // Retrying a pending debit with the same Idempotency-Key yields the same - // hold ID, so the ledger request body is identical and no duplicate hold - // is created. + // With a stable (explicit, non-expiring) source set, retrying a pending + // debit under the same Idempotency-Key yields the same hold ID *and* a + // byte-identical ledger request body — so the ledger sees a genuine replay, + // not a conflict, and no duplicate hold is created. require.Equal(t, debit().ID, debit().ID) + require.Len(t, bodies, 2) + require.Equal(t, bodies[0], bodies[1]) } diff --git a/pkg/manager.go b/pkg/manager.go index 8cc189a..9812a10 100644 --- a/pkg/manager.go +++ b/pkg/manager.go @@ -140,6 +140,16 @@ func (m *Manager) Debit(ctx context.Context, ik string, debit Debit) (*DebitHold // Derive the hold ID from the Idempotency-Key so a retry produces an // identical ledger request (the ledger hashes the body to enforce // idempotency) and returns the same hold. + // + // NOTE: this stabilises the hold ID, but not necessarily the whole + // ledger body. The source set below is resolved against live ledger + // state and filtered with time.Now(): a pending debit using + // balances:["*"] or a named balance that expires between two attempts + // can still yield a different script on retry and be rejected. Full + // idempotency therefore holds only for an explicit, non-expiring source + // set; the wildcard/expiry case needs deterministic source resolution + // (e.g. persisting the resolved request per key) and is tracked + // separately. if ik != "" { hold.ID = deterministicID(ik) } From ccf7dca9891529b4fa7eaf505ff48b611c0d85b4 Mon Sep 17 00:00:00 2001 From: Maxence Maireaux Date: Fri, 12 Jun 2026 10:06:19 +0200 Subject: [PATCH 3/3] test: cover expiring balance idempotency limit --- pkg/api/handler_wallets_debit_test.go | 61 +++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/pkg/api/handler_wallets_debit_test.go b/pkg/api/handler_wallets_debit_test.go index 20f5f17..6842e26 100644 --- a/pkg/api/handler_wallets_debit_test.go +++ b/pkg/api/handler_wallets_debit_test.go @@ -455,3 +455,64 @@ func TestWalletsDebitPendingIdempotency(t *testing.T) { require.Len(t, bodies, 2) require.Equal(t, bodies[0], bodies[1]) } + +func TestWalletsDebitPendingIdempotencyWithExpiringBalance(t *testing.T) { + t.Parallel() + + const idempotencyKey = "debit-pending-expiring-key-1" + walletID := uuid.NewString() + + var ( + getAccountCalls int + bodies []wallet.PostTransaction + testEnv *testEnv + ) + testEnv = newTestEnv( + WithGetAccount(func(ctx context.Context, ledger, account string) (*wallet.AccountWithVolumesAndBalances, error) { + require.Equal(t, testEnv.LedgerName(), ledger) + require.Equal(t, testEnv.Chart().GetBalanceAccount(walletID, "promo"), account) + + getAccountCalls++ + expiresAt := time.Now().Add(time.Hour) + if getAccountCalls > 1 { + expiresAt = time.Now().Add(-time.Hour) + } + + return &wallet.AccountWithVolumesAndBalances{ + Account: wallet.Account{ + Address: account, + Metadata: metadataWithExpectingTypesAfterUnmarshalling(wallet.Balance{ + Name: "promo", + ExpiresAt: ptr(expiresAt), + }.LedgerMetadata(walletID)), + }, + }, nil + }), + WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) { + require.Equal(t, idempotencyKey, ik) + bodies = append(bodies, p) + //nolint:nilnil + return nil, nil + }), + ) + + debit := func() { + req := newRequest(t, http.MethodPost, "/wallets/"+walletID+"/debit", wallet.DebitRequest{ + Amount: wallet.NewMonetary(big.NewInt(100), "USD"), + Pending: true, + Balances: []string{"promo"}, + }) + req.Header.Set("Idempotency-Key", idempotencyKey) + rec := httptest.NewRecorder() + testEnv.Router().ServeHTTP(rec, req) + require.Equal(t, http.StatusCreated, rec.Result().StatusCode) + } + + // This covers the limitation documented in Manager.Debit: source + // resolution still depends on live ledger state and time.Now(), so crossing + // an expiry boundary changes the ledger body even with the same key. + debit() + debit() + require.Len(t, bodies, 2) + require.NotEqual(t, bodies[0], bodies[1]) +}