Skip to content
This repository was archived by the owner on Jul 1, 2024. It is now read-only.

Commit 713f062

Browse files
0xBuzzLightyear0xBuzzLightyear
committed
Fix withdrawals blocked by a failed item in a batch
1 parent 221661c commit 713f062

File tree

4 files changed

+153
-122
lines changed

4 files changed

+153
-122
lines changed

‎internal/service/payment/service.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,11 @@ func (s *Service) Update(ctx context.Context, merchantID, id int64, props Update
580580
return s.entryToPayment(pt)
581581
}
582582

583+
func (s *Service) Fail(ctx context.Context, pt *Payment) error {
584+
_, err := s.Update(ctx, pt.MerchantID, pt.ID, UpdateProps{Status: StatusFailed})
585+
return err
586+
}
587+
583588
func (s *Service) SetWebhookTimestamp(ctx context.Context, merchantID, id int64, sentAt time.Time) error {
584589
err := s.repo.UpdatePaymentWebhookInfo(ctx, repository.UpdatePaymentWebhookInfoParams{
585590
ID: id,

‎internal/service/payment/service_withdrawal.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package payment
22

33
import (
44
"context"
5+
"fmt"
56
"strconv"
67
"time"
78

@@ -33,7 +34,7 @@ func (s *Service) ListWithdrawals(ctx context.Context, status Status, filterByID
3334
}
3435

3536
if len(filterByIDs) > 0 && len(results) != len(filterByIDs) {
36-
return nil, errors.New("results len mismatch")
37+
return nil, fmt.Errorf("withdrawals filter mismatch for status %q", status)
3738
}
3839

3940
payments := make([]*Payment, len(results))

‎internal/service/processing/service_withdrawal.go‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,16 @@ func (s *Service) BatchCreateWithdrawals(ctx context.Context, withdrawalIDs []in
2323
return nil, err
2424
}
2525

26-
// 1.Validate payments
27-
if errValidate := s.validateWithdrawals(withdrawals); errValidate != nil {
28-
return nil, errValidate
29-
}
30-
31-
// 2. Get OUTBOUND wallets and balances
26+
// 1. Get OUTBOUND wallets and balances
3227
outboundWallets, outboundBalances, err := s.getOutboundWalletsWithBalancesAsMap(ctx)
3328
if err != nil {
3429
return nil, errors.Wrap(err, "unable to get outbound wallets with balances")
3530
}
3631

3732
result := &TransferResult{}
3833

39-
// 3. For each withdrawal:
34+
// 2. For each withdrawal:
35+
// - Validate
4036
// - Resolve currency
4137
// - Resolve outbound system wallet & balance
4238
// - Resolve merchant balance & withdrawal address
@@ -47,6 +43,18 @@ func (s *Service) BatchCreateWithdrawals(ctx context.Context, withdrawalIDs []in
4743
for i := range withdrawals {
4844
withdrawal := withdrawals[i]
4945
group.Go(func() error {
46+
// Let's validate each withdrawal individually.
47+
// By doing so, we can reject it without blocking other withdrawals.
48+
if errValidate := validateWithdrawal(withdrawal); errValidate != nil {
49+
if errUpdate := s.payments.Fail(ctx, withdrawal); errUpdate != nil {
50+
result.registerErr(errors.Wrap(errUpdate, "unable to mark invalid withdrawal as failed"))
51+
} else {
52+
result.registerErr(errors.Wrap(errValidate, "withdrawal is invalid, marked as failed"))
53+
}
54+
55+
return nil
56+
}
57+
5058
currency, err := s.blockchain.GetCurrencyByTicker(withdrawal.Price.Ticker())
5159
if err != nil {
5260
result.registerErr(errors.Wrap(err, "unable to get withdrawal currency"))
@@ -540,27 +548,26 @@ func (s *Service) cancelWithdrawal(
540548
return nil
541549
}
542550

543-
func (s *Service) validateWithdrawals(withdrawals []*payment.Payment) error {
544-
for _, pt := range withdrawals {
545-
if pt.Type != payment.TypeWithdrawal {
546-
return errors.Wrap(ErrInvalidInput, "payment is not withdrawal")
547-
}
551+
func validateWithdrawal(pt *payment.Payment) error {
552+
if pt.Type != payment.TypeWithdrawal {
553+
return errors.Wrap(ErrInvalidInput, "payment is not withdrawal")
554+
}
548555

549-
if pt.Status != payment.StatusPending {
550-
return errors.Wrap(ErrInvalidInput, "withdrawal is not pending")
551-
}
556+
if pt.Status != payment.StatusPending {
557+
return errors.Wrap(ErrInvalidInput, "withdrawal is not pending")
558+
}
552559

553-
if pt.MerchantID == 0 {
554-
return errors.Wrap(ErrInvalidInput, "invalid merchant id")
555-
}
560+
if pt.MerchantID == 0 {
561+
return errors.Wrap(ErrInvalidInput, "invalid merchant id")
562+
}
556563

557-
if pt.WithdrawalBalanceID() < 1 {
558-
return errors.Wrap(ErrInvalidInput, "invalid balance id")
559-
}
564+
if pt.WithdrawalBalanceID() < 1 {
565+
return errors.Wrap(ErrInvalidInput, "invalid balance id")
566+
}
560567

561-
if pt.WithdrawalAddressID() < 1 {
562-
return errors.Wrap(ErrInvalidInput, "invalid address id")
563-
}
568+
// edge-case: a customer can delete the address while withdrawal is pending
569+
if pt.WithdrawalAddressID() < 1 {
570+
return errors.Wrap(ErrInvalidInput, "invalid address id")
564571
}
565572

566573
return nil

‎internal/service/processing/service_withdrawal_test.go‎

Lines changed: 115 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/oxygenpay/oxygen/internal/service/blockchain"
1313
"github.com/oxygenpay/oxygen/internal/service/merchant"
1414
"github.com/oxygenpay/oxygen/internal/service/payment"
15+
"github.com/oxygenpay/oxygen/internal/service/processing"
1516
"github.com/oxygenpay/oxygen/internal/service/transaction"
1617
"github.com/oxygenpay/oxygen/internal/service/wallet"
1718
"github.com/oxygenpay/oxygen/internal/test"
@@ -504,7 +505,7 @@ func TestService_BatchCreateWithdrawals(t *testing.T) {
504505
})
505506
})
506507

507-
t.Run("Creates 2 ETH transactions, one failed", func(t *testing.T) {
508+
t.Run("Creates 2 ETH transactions, one fails due to insufficient balance", func(t *testing.T) {
508509
tc.Clear.Wallets(t)
509510
isTest := false
510511

@@ -607,112 +608,129 @@ func TestService_BatchCreateWithdrawals(t *testing.T) {
607608
assert.Equal(t, expectedWalletBalance, outboundBalance.Amount.StringRaw())
608609
})
609610

610-
t.Run("Fails", func(t *testing.T) {
611+
t.Run("Validation error", func(t *testing.T) {
611612
tc.Clear.Wallets(t)
612-
t.Run("Validation error", func(t *testing.T) {
613-
mt, _ := tc.Must.CreateMerchant(t, 1)
613+
mt, _ := tc.Must.CreateMerchant(t, 1)
614614

615-
merchantBalance := tc.Must.CreateBalance(t, wallet.EntityTypeMerchant, mt.ID, withBalance(eth, "123_000_000_000_000_000", false))
616-
merchantAddress, err := tc.Services.Merchants.CreateMerchantAddress(ctx, mt.ID, merchant.CreateMerchantAddressParams{
617-
Name: "A1",
618-
Blockchain: kmswallet.Blockchain(eth.Blockchain),
619-
Address: "0x95222290dd7278aa3ddd389cc1e1d165cc4bafe5",
615+
merchantBalance := tc.Must.CreateBalance(t, wallet.EntityTypeMerchant, mt.ID, withBalance(eth, "123_000_000_000_000_000", false))
616+
merchantAddress, err := tc.Services.Merchants.CreateMerchantAddress(ctx, mt.ID, merchant.CreateMerchantAddressParams{
617+
Name: "A1",
618+
Blockchain: kmswallet.Blockchain(eth.Blockchain),
619+
Address: "0x95222290dd7278aa3ddd389cc1e1d165cc4bafe5",
620+
})
621+
require.NoError(t, err)
622+
623+
makeWithdrawal := func(amount money.Money, meta payment.Metadata) *payment.Payment {
624+
entry, err := tc.Repository.CreatePayment(ctx, repository.CreatePaymentParams{
625+
PublicID: uuid.New(),
626+
CreatedAt: time.Now(),
627+
UpdatedAt: time.Now(),
628+
Type: string(payment.TypeWithdrawal),
629+
Status: string(payment.StatusPending),
630+
MerchantID: mt.ID,
631+
MerchantOrderUuid: uuid.New(),
632+
Price: repository.MoneyToNumeric(amount),
633+
Decimals: int32(amount.Decimals()),
634+
Currency: amount.Ticker(),
635+
Metadata: meta.ToJSONB(),
636+
IsTest: false,
620637
})
621638
require.NoError(t, err)
622639

623-
makeWithdrawal := func(amount money.Money, meta payment.Metadata) *payment.Payment {
624-
entry, err := tc.Repository.CreatePayment(ctx, repository.CreatePaymentParams{
625-
PublicID: uuid.New(),
626-
CreatedAt: time.Now(),
627-
UpdatedAt: time.Now(),
628-
Type: string(payment.TypeWithdrawal),
629-
Status: string(payment.StatusPending),
630-
MerchantID: mt.ID,
631-
MerchantOrderUuid: uuid.New(),
632-
Price: repository.MoneyToNumeric(amount),
633-
Decimals: int32(amount.Decimals()),
634-
Currency: amount.Ticker(),
635-
Metadata: meta.ToJSONB(),
636-
IsTest: false,
637-
})
638-
require.NoError(t, err)
639-
640-
pt, err := tc.Services.Payment.GetByID(ctx, mt.ID, entry.ID)
641-
require.NoError(t, err)
642-
643-
return pt
644-
}
645-
646-
for testCaseIndex, testCase := range []struct {
647-
errContains string
648-
withdrawals func() []*payment.Payment
649-
}{
650-
{
651-
// actually "payment is not withdrawal"
652-
errContains: "results len mismatch",
653-
withdrawals: func() []*payment.Payment {
654-
return []*payment.Payment{tc.CreateSamplePayment(t, mt.ID)}
655-
},
640+
pt, err := tc.Services.Payment.GetByID(ctx, mt.ID, entry.ID)
641+
require.NoError(t, err)
642+
643+
return pt
644+
}
645+
646+
for _, tt := range []struct {
647+
name string
648+
assert func(t *testing.T, in []*payment.Payment, result *processing.TransferResult, err error)
649+
withdrawals func() []*payment.Payment
650+
}{
651+
{
652+
name: "payment is not withdrawal",
653+
withdrawals: func() []*payment.Payment {
654+
return []*payment.Payment{tc.CreateSamplePayment(t, mt.ID)}
656655
},
657-
{
658-
// actually "status is not pending"
659-
errContains: "results len mismatch",
660-
withdrawals: func() []*payment.Payment {
661-
withdrawal, err := tc.Services.Payment.CreateWithdrawal(ctx, mt.ID, payment.CreateWithdrawalProps{
662-
BalanceID: merchantBalance.UUID,
663-
AddressID: merchantAddress.UUID,
664-
AmountRaw: "0.1",
665-
})
666-
require.NoError(t, err)
667-
668-
_, err = tc.Services.Payment.Update(ctx, mt.ID, withdrawal.ID, payment.UpdateProps{Status: payment.StatusInProgress})
669-
require.NoError(t, err)
670-
671-
return []*payment.Payment{withdrawal}
672-
},
656+
assert: func(t *testing.T, _ []*payment.Payment, _ *processing.TransferResult, err error) {
657+
assert.ErrorContains(t, err, `withdrawals filter mismatch for status "pending"`)
673658
},
674-
{
675-
errContains: "invalid address id",
676-
withdrawals: func() []*payment.Payment {
677-
return []*payment.Payment{
678-
makeWithdrawal(
679-
lo.Must(eth.MakeAmount("123_456")),
680-
payment.Metadata{payment.MetaBalanceID: strconv.Itoa(int(merchantBalance.ID))},
681-
),
682-
}
683-
},
659+
},
660+
{
661+
name: "status is not pending",
662+
withdrawals: func() []*payment.Payment {
663+
withdrawal, err := tc.Services.Payment.CreateWithdrawal(ctx, mt.ID, payment.CreateWithdrawalProps{
664+
BalanceID: merchantBalance.UUID,
665+
AddressID: merchantAddress.UUID,
666+
AmountRaw: "0.1",
667+
})
668+
require.NoError(t, err)
669+
670+
_, err = tc.Services.Payment.Update(ctx, mt.ID, withdrawal.ID, payment.UpdateProps{Status: payment.StatusInProgress})
671+
require.NoError(t, err)
672+
673+
return []*payment.Payment{withdrawal}
684674
},
685-
{
686-
errContains: "invalid balance id",
687-
withdrawals: func() []*payment.Payment {
688-
return []*payment.Payment{
689-
makeWithdrawal(
690-
lo.Must(eth.MakeAmount("123_456")),
691-
payment.Metadata{payment.MetaAddressID: strconv.Itoa(int(merchantAddress.ID))},
692-
),
693-
}
694-
},
675+
assert: func(t *testing.T, _ []*payment.Payment, _ *processing.TransferResult, err error) {
676+
assert.ErrorContains(t, err, `withdrawals filter mismatch for status "pending"`)
695677
},
696-
} {
697-
t.Run(strconv.Itoa(testCaseIndex+1), func(t *testing.T) {
698-
// ARRANGE
699-
// Given balances
700-
input := testCase.withdrawals()
701-
702-
// ACT
703-
// Transfer money
704-
ids := util.MapSlice(input, func(p *payment.Payment) int64 { return p.ID })
705-
result, err := tc.Services.Processing.BatchCreateWithdrawals(ctx, ids)
706-
assert.Nil(t, result)
707-
708-
// ASSERT
709-
// Check that error contain string
710-
if testCase.errContains != "" {
711-
assert.ErrorContains(t, err, testCase.errContains)
678+
},
679+
{
680+
name: "invalid address id",
681+
withdrawals: func() []*payment.Payment {
682+
return []*payment.Payment{
683+
makeWithdrawal(
684+
lo.Must(eth.MakeAmount("123_456")),
685+
payment.Metadata{payment.MetaBalanceID: strconv.Itoa(int(merchantBalance.ID))},
686+
),
712687
}
713-
})
714-
}
715-
})
688+
},
689+
assert: func(t *testing.T, in []*payment.Payment, result *processing.TransferResult, err error) {
690+
assert.NoError(t, err)
691+
assert.Equal(t, int64(1), result.TotalErrors)
692+
assert.ErrorContains(t, result.UnhandledErrors[0], "withdrawal is invalid, marked as failed")
693+
694+
pt, _ := tc.Services.Payment.GetByID(ctx, in[0].MerchantID, in[0].ID)
695+
require.NoError(t, err)
696+
assert.Equal(t, payment.StatusFailed, pt.Status)
697+
},
698+
},
699+
{
700+
name: "invalid balance id",
701+
withdrawals: func() []*payment.Payment {
702+
return []*payment.Payment{
703+
makeWithdrawal(
704+
lo.Must(eth.MakeAmount("123_456")),
705+
payment.Metadata{payment.MetaAddressID: strconv.Itoa(int(merchantAddress.ID))},
706+
),
707+
}
708+
},
709+
assert: func(t *testing.T, in []*payment.Payment, result *processing.TransferResult, err error) {
710+
assert.NoError(t, err)
711+
assert.Equal(t, int64(1), result.TotalErrors)
712+
assert.ErrorContains(t, result.UnhandledErrors[0], "withdrawal is invalid, marked as failed")
713+
714+
pt, _ := tc.Services.Payment.GetByID(ctx, in[0].MerchantID, in[0].ID)
715+
require.NoError(t, err)
716+
assert.Equal(t, payment.StatusFailed, pt.Status)
717+
},
718+
},
719+
} {
720+
t.Run(tt.name, func(t *testing.T) {
721+
// ARRANGE
722+
// Given balances
723+
input := tt.withdrawals()
724+
725+
// ACT
726+
// Create withdrawals
727+
ids := util.MapSlice(input, func(p *payment.Payment) int64 { return p.ID })
728+
result, err := tc.Services.Processing.BatchCreateWithdrawals(ctx, ids)
729+
730+
// ASSERT
731+
tt.assert(t, input, result, err)
732+
})
733+
}
716734
})
717735

718736
t.Run("Logic error", func(t *testing.T) {

0 commit comments

Comments
 (0)