feat: add 'force' query param to force reverts even if the source acc…

…ounts does not have enough funds (#829)
formancehq · Dec 4, 2023 · b7c7080 · b7c7080
1 parent c331e81
commit b7c7080
Show file tree

Hide file tree

Showing 24 changed files with 184 additions and 88 deletions.
diff --git a/internal/api/backend/backend.go b/internal/api/backend/backend.go
@@ -29,7 +29,7 @@ type Ledger interface {
 	GetTransactionWithVolumes(ctx context.Context, query ledgerstore.GetTransactionQuery) (*ledger.ExpandedTransaction, error)
 
 	CreateTransaction(ctx context.Context, parameters command.Parameters, data ledger.RunScript) (*ledger.Transaction, error)
-	RevertTransaction(ctx context.Context, parameters command.Parameters, id *big.Int) (*ledger.Transaction, error)
+	RevertTransaction(ctx context.Context, parameters command.Parameters, id *big.Int, force bool) (*ledger.Transaction, error)
 	SaveMeta(ctx context.Context, parameters command.Parameters, targetType string, targetID any, m metadata.Metadata) error
 	DeleteMetadata(ctx context.Context, parameters command.Parameters, targetType string, targetID any, key string) error
 

diff --git a/internal/api/backend/backend_generated.go b/internal/api/backend/backend_generated.go
diff --git a/internal/api/bulk/bulk.go b/internal/api/bulk/bulk.go
@@ -114,14 +114,15 @@ func ProcessBulk(ctx context.Context, l backend.Ledger, bulk Bulk, continueOnFai
 			}
 		case ActionRevertTransaction:
 			type revertTransactionRequest struct {
-				ID *big.Int `json:"id"`
+				ID    *big.Int `json:"id"`
+				Force bool     `json:"force"`
 			}
 			req := &revertTransactionRequest{}
 			if err := json.Unmarshal(element.Data, req); err != nil {
 				return nil, errorsInBulk, fmt.Errorf("error parsing element %d: %s", i, err)
 			}
 
-			tx, err := l.RevertTransaction(ctx, parameters, req.ID)
+			tx, err := l.RevertTransaction(ctx, parameters, req.ID, req.Force)
 			if err != nil {
 				bulkError(element.Action, err)
 				if !continueOnFailure {

diff --git a/internal/api/v1/controllers_transactions.go b/internal/api/v1/controllers_transactions.go
@@ -202,7 +202,7 @@ func postTransaction(w http.ResponseWriter, r *http.Request) {
 			Metadata:  payload.Metadata,
 		}
 
-		res, err := l.CreateTransaction(r.Context(), getCommandParameters(r), ledger.TxToScriptData(txData))
+		res, err := l.CreateTransaction(r.Context(), getCommandParameters(r), ledger.TxToScriptData(txData, false))
 		if err != nil {
 			ResponseError(w, r, err)
 			return
@@ -264,7 +264,7 @@ func revertTransaction(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	tx, err := l.RevertTransaction(r.Context(), getCommandParameters(r), transactionID)
+	tx, err := l.RevertTransaction(r.Context(), getCommandParameters(r), transactionID, sharedapi.QueryParamBool(r, "disableChecks"))
 	if err != nil {
 		ResponseError(w, r, err)
 		return

diff --git a/internal/api/v1/controllers_transactions_test.go b/internal/api/v1/controllers_transactions_test.go
@@ -156,7 +156,7 @@ func TestPostTransactions(t *testing.T) {
 			},
 			expectedRunScript: ledger.TxToScriptData(ledger.NewTransactionData().WithPostings(
 				ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
-			)),
+			), false),
 		},
 		{
 			name: "using JSON postings and dry run",
@@ -171,7 +171,7 @@ func TestPostTransactions(t *testing.T) {
 			expectedPreview: true,
 			expectedRunScript: ledger.TxToScriptData(ledger.NewTransactionData().WithPostings(
 				ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
-			)),
+			), false),
 		},
 		{
 			name:               "no postings or script",
@@ -608,7 +608,7 @@ func TestRevertTransaction(t *testing.T) {
 	backend, mockLedger := newTestingBackend(t, true)
 	mockLedger.
 		EXPECT().
-		RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(0)).
+		RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(0), false).
 		Return(expectedTx, nil)
 
 	router := v1.NewRouter(backend, nil, metrics.NewNoOpRegistry())
@@ -623,3 +623,28 @@ func TestRevertTransaction(t *testing.T) {
 	require.True(t, ok)
 	require.Equal(t, *expectedTx, tx)
 }
+
+func TestForceRevertTransaction(t *testing.T) {
+
+	expectedTx := ledger.NewTransaction().WithPostings(
+		ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
+	)
+
+	backend, mockLedger := newTestingBackend(t, true)
+	mockLedger.
+		EXPECT().
+		RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(0), true).
+		Return(expectedTx, nil)
+
+	router := v1.NewRouter(backend, nil, metrics.NewNoOpRegistry())
+
+	req := httptest.NewRequest(http.MethodPost, "/xxx/transactions/0/revert?disableChecks=true", nil)
+	rec := httptest.NewRecorder()
+
+	router.ServeHTTP(rec, req)
+
+	require.Equal(t, http.StatusCreated, rec.Code)
+	tx, ok := sharedapi.DecodeSingleResponse[ledger.Transaction](t, rec.Body)
+	require.True(t, ok)
+	require.Equal(t, *expectedTx, tx)
+}
diff --git a/internal/api/v2/controllers_bulk_test.go b/internal/api/v2/controllers_bulk_test.go
@@ -63,7 +63,7 @@ func TestBulk(t *testing.T) {
 					CreateTransaction(gomock.Any(), command.Parameters{}, ledger.TxToScriptData(ledger.TransactionData{
 						Postings:  postings,
 						Timestamp: now,
-					})).
+					}, false)).
 					Return(&ledger.Transaction{
 						TransactionData: ledger.TransactionData{
 							Postings:  postings,
@@ -147,7 +147,7 @@ func TestBulk(t *testing.T) {
 			}]`,
 			expectations: func(mockLedger *backend.MockLedger) {
 				mockLedger.EXPECT().
-					RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(1)).
+					RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(1), false).
 					Return(&ledger.Transaction{}, nil)
 			},
 			expectResults: []bulk.Result{{

diff --git a/internal/api/v2/controllers_transactions.go b/internal/api/v2/controllers_transactions.go
@@ -138,7 +138,7 @@ func revertTransaction(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	tx, err := l.RevertTransaction(r.Context(), getCommandParameters(r), transactionID)
+	tx, err := l.RevertTransaction(r.Context(), getCommandParameters(r), transactionID, sharedapi.QueryParamBool(r, "force"))
 	if err != nil {
 		shared.ResponseError(w, r, err)
 		return

diff --git a/internal/api/v2/controllers_transactions_test.go b/internal/api/v2/controllers_transactions_test.go
@@ -161,7 +161,7 @@ func TestPostTransactions(t *testing.T) {
 			},
 			expectedRunScript: ledger.TxToScriptData(ledger.NewTransactionData().WithPostings(
 				ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
-			)),
+			), false),
 		},
 		{
 			name: "using JSON postings and dry run",
@@ -176,7 +176,7 @@ func TestPostTransactions(t *testing.T) {
 			expectedDryRun: true,
 			expectedRunScript: ledger.TxToScriptData(ledger.NewTransactionData().WithPostings(
 				ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
-			)),
+			), false),
 		},
 		{
 			name:               "no postings or script",
@@ -671,7 +671,7 @@ func TestRevertTransaction(t *testing.T) {
 	backend, mockLedger := newTestingBackend(t, true)
 	mockLedger.
 		EXPECT().
-		RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(0)).
+		RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(0), false).
 		Return(expectedTx, nil)
 
 	router := v2.NewRouter(backend, nil, metrics.NewNoOpRegistry())
@@ -686,3 +686,28 @@ func TestRevertTransaction(t *testing.T) {
 	require.True(t, ok)
 	require.Equal(t, *expectedTx, tx)
 }
+
+func TestForceRevertTransaction(t *testing.T) {
+
+	expectedTx := ledger.NewTransaction().WithPostings(
+		ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
+	)
+
+	backend, mockLedger := newTestingBackend(t, true)
+	mockLedger.
+		EXPECT().
+		RevertTransaction(gomock.Any(), command.Parameters{}, big.NewInt(0), true).
+		Return(expectedTx, nil)
+
+	router := v2.NewRouter(backend, nil, metrics.NewNoOpRegistry())
+
+	req := httptest.NewRequest(http.MethodPost, "/xxx/transactions/0/revert?force=true", nil)
+	rec := httptest.NewRecorder()
+
+	router.ServeHTTP(rec, req)
+
+	require.Equal(t, http.StatusCreated, rec.Code)
+	tx, ok := sharedapi.DecodeSingleResponse[ledger.Transaction](t, rec.Body)
+	require.True(t, ok)
+	require.Equal(t, *expectedTx, tx)
+}
diff --git a/internal/engine/command/commander.go b/internal/engine/command/commander.go
@@ -218,7 +218,7 @@ func (commander *Commander) SaveMeta(ctx context.Context, parameters Parameters,
 	return nil
 }
 
-func (commander *Commander) RevertTransaction(ctx context.Context, parameters Parameters, id *big.Int) (*ledger.Transaction, error) {
+func (commander *Commander) RevertTransaction(ctx context.Context, parameters Parameters, id *big.Int, force bool) (*ledger.Transaction, error) {
 
 	if err := commander.referencer.take(referenceReverts, id); err != nil {
 		return nil, ErrRevertOccurring
@@ -251,7 +251,7 @@ func (commander *Commander) RevertTransaction(ctx context.Context, parameters Pa
 		ledger.TxToScriptData(ledger.TransactionData{
 			Postings: rt.Postings,
 			Metadata: rt.Metadata,
-		}),
+		}, force),
 		func(tx *ledger.Transaction, accountMetadata map[string]metadata.Metadata) *ledger.Log {
 			return ledger.NewRevertedTransactionLog(tx.Timestamp, transactionToRevert.ID, tx)
 		})

diff --git a/internal/engine/command/commander_test.go b/internal/engine/command/commander_test.go
@@ -205,7 +205,7 @@ func TestRevert(t *testing.T) {
 	go commander.Run(ctx)
 	defer commander.Close()
 
-	_, err = commander.RevertTransaction(ctx, Parameters{}, txID)
+	_, err = commander.RevertTransaction(ctx, Parameters{}, txID, false)
 	require.NoError(t, err)
 }
 
@@ -225,7 +225,7 @@ func TestRevertWithAlreadyReverted(t *testing.T) {
 	go commander.Run(ctx)
 	defer commander.Close()
 
-	_, err = commander.RevertTransaction(context.Background(), Parameters{}, tx.ID)
+	_, err = commander.RevertTransaction(context.Background(), Parameters{}, tx.ID, false)
 	require.True(t, errors.Is(err, ErrAlreadyReverted))
 }
 
@@ -248,6 +248,47 @@ func TestRevertWithRevertOccurring(t *testing.T) {
 
 	referencer.take(referenceReverts, big.NewInt(0))
 
-	_, err = commander.RevertTransaction(ctx, Parameters{}, tx.ID)
+	_, err = commander.RevertTransaction(ctx, Parameters{}, tx.ID, false)
 	require.True(t, errors.Is(err, ErrRevertOccurring))
 }
+
+func TestForceRevert(t *testing.T) {
+
+	store := storageerrors.NewInMemoryStore()
+	ctx := logging.TestingContext()
+
+	tx1 := ledger.NewTransaction().WithPostings(
+		ledger.NewPosting("world", "bank", "USD", big.NewInt(100)),
+	)
+	tx2 := ledger.NewTransaction().WithPostings(
+		ledger.NewPosting("bank", "foo", "USD", big.NewInt(100)),
+	)
+	err := store.InsertLogs(ctx, ledger.ChainLogs(
+		ledger.NewTransactionLog(tx1, map[string]metadata.Metadata{}),
+		ledger.NewTransactionLog(tx2, map[string]metadata.Metadata{}),
+	)...)
+	require.NoError(t, err)
+
+	commander := New(store, NoOpLocker, NewCompiler(1024), NewReferencer(), bus.NewNoOpMonitor())
+	go commander.Run(ctx)
+	defer commander.Close()
+
+	_, err = commander.RevertTransaction(ctx, Parameters{}, tx1.ID, false)
+	require.NotNil(t, err)
+	require.True(t, errors.Is(err, ledger.ErrInsufficientFund))
+
+	balance, err := store.GetBalance(ctx, "bank", "USD")
+	require.NoError(t, err)
+	require.Equal(t, uint64(0), balance.Uint64())
+
+	_, err = commander.RevertTransaction(ctx, Parameters{}, tx1.ID, true)
+	require.Nil(t, err)
+
+	balance, err = store.GetBalance(ctx, "bank", "USD")
+	require.NoError(t, err)
+	require.Equal(t, big.NewInt(-100), balance)
+
+	balance, err = store.GetBalance(ctx, "world", "USD")
+	require.NoError(t, err)
+	require.Equal(t, uint64(0), balance.Uint64())
+}
diff --git a/internal/engine/ledger.go b/internal/engine/ledger.go
@@ -98,8 +98,8 @@ func (l *Ledger) CreateTransaction(ctx context.Context, parameters command.Param
 	return l.commander.CreateTransaction(ctx, parameters, data)
 }
 
-func (l *Ledger) RevertTransaction(ctx context.Context, parameters command.Parameters, id *big.Int) (*ledger.Transaction, error) {
-	return l.commander.RevertTransaction(ctx, parameters, id)
+func (l *Ledger) RevertTransaction(ctx context.Context, parameters command.Parameters, id *big.Int, force bool) (*ledger.Transaction, error) {
+	return l.commander.RevertTransaction(ctx, parameters, id, force)
 }
 
 func (l *Ledger) SaveMeta(ctx context.Context, parameters command.Parameters, targetType string, targetID any, m metadata.Metadata) error {

diff --git a/internal/errors.go b/internal/errors.go
@@ -0,0 +1,5 @@
+package ledger
+
+import "github.com/pkg/errors"
+
+var ErrInsufficientFund = errors.New("account had insufficient funds")
diff --git a/internal/machine/internal/funding.go b/internal/machine/internal/funding.go
@@ -3,6 +3,8 @@ package internal
 import (
 	"errors"
 	"fmt"
+
+	ledger "github.com/formancehq/ledger/internal"
 )
 
 type FundingPart struct {
@@ -89,7 +91,7 @@ func (f Funding) Take(amount *MonetaryInt) (Funding, Funding, error) {
 		i++
 	}
 	if !remainingToWithdraw.Eq(Zero) {
-		return Funding{}, Funding{}, errors.New("account had insufficient funds")
+		return Funding{}, Funding{}, ledger.ErrInsufficientFund
 	}
 	return result, remainder, nil
 }

diff --git a/internal/machine/machine.go b/internal/machine/machine.go
@@ -4,7 +4,7 @@ import (
 	"math/big"
 
 	ledger "github.com/formancehq/ledger/internal"
-	vm2 "github.com/formancehq/ledger/internal/machine/vm"
+	"github.com/formancehq/ledger/internal/machine/vm"
 	"github.com/formancehq/stack/libs/go-libs/errorsutil"
 	"github.com/formancehq/stack/libs/go-libs/metadata"
 	"github.com/pkg/errors"
@@ -16,7 +16,7 @@ type Result struct {
 	AccountMetadata map[string]metadata.Metadata
 }
 
-func Run(m *vm2.Machine, script ledger.RunScript) (*Result, error) {
+func Run(m *vm.Machine, script ledger.RunScript) (*Result, error) {
 	err := m.Execute()
 	if err != nil {
 		return nil, errors.Wrap(err, "script execution failed")
@@ -40,7 +40,7 @@ func Run(m *vm2.Machine, script ledger.RunScript) (*Result, error) {
 	for k, v := range script.Metadata {
 		_, ok := result.Metadata[k]
 		if ok {
-			return nil, errorsutil.NewError(vm2.ErrMetadataOverride,
+			return nil, errorsutil.NewError(vm.ErrMetadataOverride,
 				errors.New("cannot override metadata from script"))
 		}
 		result.Metadata[k] = v

diff --git a/internal/machine/machine_test.go b/internal/machine/machine_test.go
@@ -46,7 +46,7 @@ var testCases = []testCase{
 				source = @bank
 				destination = @user:001
 			)`,
-		expectErrorCode: vm2.ErrInsufficientFund,
+		expectErrorCode: ledger.ErrInsufficientFund,
 	},
 	{
 		name: "send $0",