fix: make wallet creation and pending debits idempotent (EN-1204)

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

pkg/api/handler_wallets_create_test.go

@@ -51,3 +51,47 @@ 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"
+
+	// 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", request)
+		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
+	// (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])
+}

pkg/api/handler_wallets_debit_test.go

@@ -416,3 +416,103 @@ func TestWalletsDebit(t *testing.T) {
 		})
 	}
 }
+
+func TestWalletsDebitPendingIdempotency(t *testing.T) {
+	t.Parallel()
+
+	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
+		}),
+	)
+
+	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
+	}
+
+	// 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])
+}
+
+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])
+}

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,22 @@ 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.
+		//
+		// 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)
+		}
 		holdAccount := m.chart.GetHoldAccount(hold.ID)
 		metadata = map[string]map[string]string{
 			holdAccount: hold.LedgerMetadata(m.chart),
@@ -418,14 +445,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")