Skip to content

Commit 2a5c287

Browse files
AndresQuijanoAndresQuijano
committed
fix: feedback comments
1 parent 7e52c59 commit 2a5c287

File tree

12 files changed

+266
-298
lines changed

12 files changed

+266
-298
lines changed

‎internal/adapters/dataproviders/database/mongo/liquidity_provider_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ var (
9191

9292
var testStateConfig = &entities.Signed[liquidity_provider.StateConfiguration]{
9393
Value: liquidity_provider.StateConfiguration{
94-
LastBtcToColdWalletTransfer: &lastBtcToColdWalletTransferUnix,
95-
LastRbtcToColdWalletTransfer: &lastRbtcToColdWalletTransferUnix,
94+
LastBtcToColdWalletTransfer: lastBtcToColdWalletTransferUnix,
95+
LastRbtcToColdWalletTransfer: lastRbtcToColdWalletTransferUnix,
9696
},
9797
Signature: "state signature",
9898
Hash: "state hash",

‎internal/adapters/dataproviders/liquidity_provider.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,14 @@ func (lp *LocalLiquidityProvider) PeginConfiguration(ctx context.Context) liquid
205205
return configuration.Value
206206
}
207207

208-
func (lp *LocalLiquidityProvider) StateConfiguration(ctx context.Context) liquidity_provider.StateConfiguration {
208+
func (lp *LocalLiquidityProvider) StateConfiguration(ctx context.Context) (liquidity_provider.StateConfiguration, error) {
209209
configuration, err := liquidity_provider.ValidateConfiguration(lp.signer, crypto.Keccak256, func() (*entities.Signed[liquidity_provider.StateConfiguration], error) {
210210
return lp.lpRepository.GetStateConfiguration(ctx)
211211
})
212212
if err != nil {
213-
lp.logConfigError("state", err)
214-
return liquidity_provider.StateConfiguration{}
213+
return liquidity_provider.StateConfiguration{}, err
215214
}
216-
return configuration.Value
215+
return configuration.Value, nil
217216
}
218217

219218
func (lp *LocalLiquidityProvider) GetSigner() entities.Signer {

‎internal/adapters/dataproviders/liquidity_provider_test.go‎

Lines changed: 35 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -727,77 +727,71 @@ func TestLocalLiquidityProvider_StateConfiguration(t *testing.T) {
727727
btcUnix := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC).Unix()
728728
rbtcUnix := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC).Unix()
729729
stateConfigValue := liquidity_provider.StateConfiguration{
730-
LastBtcToColdWalletTransfer: &btcUnix,
731-
LastRbtcToColdWalletTransfer: &rbtcUnix,
730+
LastBtcToColdWalletTransfer: btcUnix,
731+
LastRbtcToColdWalletTransfer: rbtcUnix,
732732
}
733733

734734
signedConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
735735
require.NoError(t, err)
736736

737737
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&signedConfig, nil).Once()
738738
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
739-
result := lp.StateConfiguration(context.Background())
739+
result, err := lp.StateConfiguration(context.Background())
740740

741+
require.NoError(t, err)
741742
assert.Equal(t, stateConfigValue, result)
742-
assert.NotNil(t, result.LastBtcToColdWalletTransfer)
743-
assert.NotNil(t, result.LastRbtcToColdWalletTransfer)
743+
assert.NotZero(t, result.LastBtcToColdWalletTransfer)
744+
assert.NotZero(t, result.LastRbtcToColdWalletTransfer)
744745
})
745746

746-
t.Run("Return empty state configuration when db doesn't have configuration", func(t *testing.T) {
747+
t.Run("Return error when db doesn't have configuration", func(t *testing.T) {
747748
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(nil, nil).Once()
748749
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, nil, nil, blockchain.RskContracts{})
749750

750-
defer test.AssertLogContains(t, "Custom state configuration not found. Using default configuration.")()
751-
752-
config := lp.StateConfiguration(context.Background())
751+
_, err := lp.StateConfiguration(context.Background())
753752

754-
// StateConfiguration has no defaults - returns empty struct
755-
assert.Nil(t, config.LastBtcToColdWalletTransfer)
756-
assert.Nil(t, config.LastRbtcToColdWalletTransfer)
753+
require.ErrorIs(t, err, liquidity_provider.ConfigurationNotFoundError)
757754
})
758755

759-
t.Run("Return empty state configuration on db read error", func(t *testing.T) {
756+
t.Run("Return error on db read error", func(t *testing.T) {
760757
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(nil, errors.New("database error")).Once()
761758
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, nil, nil, blockchain.RskContracts{})
762759

763-
defer test.AssertLogContains(t, "Error getting state configuration, using default configuration. Error: database error")()
764-
config := lp.StateConfiguration(context.Background())
760+
_, err := lp.StateConfiguration(context.Background())
765761

766-
assert.Nil(t, config.LastBtcToColdWalletTransfer)
767-
assert.Nil(t, config.LastRbtcToColdWalletTransfer)
762+
require.Error(t, err)
763+
assert.Contains(t, err.Error(), "database error")
768764
})
769765

770-
t.Run("Return empty state configuration when db configuration is tampered", func(t *testing.T) {
766+
t.Run("Return error when db configuration is tampered", func(t *testing.T) {
771767
btcUnix := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC).Unix()
772768
rbtcUnix := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC).Unix()
773769
stateConfigValue := liquidity_provider.StateConfiguration{
774-
LastBtcToColdWalletTransfer: &btcUnix,
775-
LastRbtcToColdWalletTransfer: &rbtcUnix,
770+
LastBtcToColdWalletTransfer: btcUnix,
771+
LastRbtcToColdWalletTransfer: rbtcUnix,
776772
}
777773

778774
tamperedConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
779775
require.NoError(t, err)
780776

781777
// Tamper with the data after signing
782778
newUnix := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC).Unix()
783-
tamperedConfig.Value.LastBtcToColdWalletTransfer = &newUnix
779+
tamperedConfig.Value.LastBtcToColdWalletTransfer = newUnix
784780

785781
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&tamperedConfig, nil).Once()
786782
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
787783

788-
defer test.AssertLogContains(t, "Tampered state configuration. Using default configuration.")()
789-
result := lp.StateConfiguration(context.Background())
784+
_, err = lp.StateConfiguration(context.Background())
790785

791-
assert.Nil(t, result.LastBtcToColdWalletTransfer)
792-
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
786+
require.ErrorIs(t, err, entities.IntegrityError)
793787
})
794788

795-
t.Run("Return empty state configuration when db configuration doesn't have valid signature", func(t *testing.T) {
789+
t.Run("Return error when db configuration doesn't have valid signature", func(t *testing.T) {
796790
btcUnix := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC).Unix()
797791
rbtcUnix := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC).Unix()
798792
stateConfigValue := liquidity_provider.StateConfiguration{
799-
LastBtcToColdWalletTransfer: &btcUnix,
800-
LastRbtcToColdWalletTransfer: &rbtcUnix,
793+
LastBtcToColdWalletTransfer: btcUnix,
794+
LastRbtcToColdWalletTransfer: rbtcUnix,
801795
}
802796

803797
// Create config with correct hash but wrong signature
@@ -809,20 +803,17 @@ func TestLocalLiquidityProvider_StateConfiguration(t *testing.T) {
809803
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&signedConfig, nil).Once()
810804
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
811805

812-
defer test.AssertLogContains(t, "Invalid state configuration signature. Using default configuration.")()
813-
814-
result := lp.StateConfiguration(context.Background())
806+
_, err = lp.StateConfiguration(context.Background())
815807

816-
assert.Nil(t, result.LastBtcToColdWalletTransfer)
817-
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
808+
require.ErrorIs(t, err, liquidity_provider.InvalidSignatureError)
818809
})
819810

820-
t.Run("Return empty state configuration when integrity check fails", func(t *testing.T) {
811+
t.Run("Return error when integrity check fails", func(t *testing.T) {
821812
btcUnix := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC).Unix()
822813
rbtcUnix := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC).Unix()
823814
stateConfigValue := liquidity_provider.StateConfiguration{
824-
LastBtcToColdWalletTransfer: &btcUnix,
825-
LastRbtcToColdWalletTransfer: &rbtcUnix,
815+
LastBtcToColdWalletTransfer: btcUnix,
816+
LastRbtcToColdWalletTransfer: rbtcUnix,
826817
}
827818

828819
integrityFailConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
@@ -834,20 +825,17 @@ func TestLocalLiquidityProvider_StateConfiguration(t *testing.T) {
834825
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&integrityFailConfig, nil).Once()
835826
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
836827

837-
defer test.AssertLogContains(t, "Tampered state configuration. Using default configuration.")()
828+
_, err = lp.StateConfiguration(context.Background())
838829

839-
result := lp.StateConfiguration(context.Background())
840-
841-
assert.Nil(t, result.LastBtcToColdWalletTransfer)
842-
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
830+
require.ErrorIs(t, err, entities.IntegrityError)
843831
})
844832

845-
t.Run("Return empty state configuration when there is an unexpected error", func(t *testing.T) {
833+
t.Run("Return error when there is an unexpected error", func(t *testing.T) {
846834
btcUnix := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC).Unix()
847835
rbtcUnix := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC).Unix()
848836
stateConfigValue := liquidity_provider.StateConfiguration{
849-
LastBtcToColdWalletTransfer: &btcUnix,
850-
LastRbtcToColdWalletTransfer: &rbtcUnix,
837+
LastBtcToColdWalletTransfer: btcUnix,
838+
LastRbtcToColdWalletTransfer: rbtcUnix,
851839
}
852840

853841
integrityFailConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
@@ -859,11 +847,9 @@ func TestLocalLiquidityProvider_StateConfiguration(t *testing.T) {
859847
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&integrityFailConfig, nil).Once()
860848
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
861849

862-
defer test.AssertLogContains(t, "Error getting state configuration, using default configuration. Error: encoding/hex: invalid byte: U+006E")()
863-
864-
result := lp.StateConfiguration(context.Background())
850+
_, err = lp.StateConfiguration(context.Background())
865851

866-
assert.Nil(t, result.LastBtcToColdWalletTransfer)
867-
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
852+
require.Error(t, err)
853+
assert.Contains(t, err.Error(), "encoding/hex: invalid byte")
868854
})
869855
}

‎internal/adapters/entrypoints/rest/routes/routes_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,6 @@ func setupRegistryMock(registryMock *mocks.UseCaseRegistryMock) {
256256
registryMock.EXPECT().DeleteTrustedAccountUseCase().Return(&liquidity_provider.DeleteTrustedAccountUseCase{})
257257
registryMock.EXPECT().RecommendedPegoutUseCase().Return(&pegout.RecommendedPegoutUseCase{})
258258
registryMock.EXPECT().RecommendedPeginUseCase().Return(&pegin.RecommendedPeginUseCase{})
259-
registryMock.EXPECT().TransferExcessToColdWalletUseCase().Return(&liquidity_provider.TransferExcessToColdWalletUseCase{}).Maybe()
260259
}
261260

262261
func assertHasCorsHeaders(t *testing.T, recorder *httptest.ResponseRecorder) {

‎internal/configuration/registry/usecase.go‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,12 @@ func NewUseCaseRegistry(
254254
databaseRegistry.LiquidityProviderRepository,
255255
messaging.EventBus,
256256
),
257-
initializeStateConfigurationUseCase: liquidity_provider.NewInitializeStateConfigurationUseCase(
258-
databaseRegistry.LiquidityProviderRepository,
259-
rskRegistry.Wallet,
260-
signingHashFunction,
261-
),
257+
initializeStateConfigurationUseCase: liquidity_provider.NewInitializeStateConfigurationUseCase(
258+
lpRegistry.LiquidityProvider,
259+
databaseRegistry.LiquidityProviderRepository,
260+
rskRegistry.Wallet,
261+
signingHashFunction,
262+
),
262263
getManagementUiDataUseCase: liquidity_provider.NewGetManagementUiDataUseCase(
263264
databaseRegistry.LiquidityProviderRepository,
264265
lpRegistry.LiquidityProvider,
@@ -369,24 +370,24 @@ func NewUseCaseRegistry(
369370
env.Rsk.FeeCollectorAddress,
370371
utils.Scale,
371372
),
372-
transferExcessToColdWalletUseCase: liquidity_provider.NewTransferExcessToColdWalletUseCase(
373-
lpRegistry.LiquidityProvider,
374-
lpRegistry.LiquidityProvider,
375-
lpRegistry.LiquidityProvider,
376-
databaseRegistry.LiquidityProviderRepository,
377-
lpRegistry.ColdWallet,
378-
btcRegistry.PaymentWallet,
379-
rskRegistry.Wallet,
380-
messaging.Rpc,
381-
mutexes.BtcWalletMutex(),
382-
mutexes.RskWalletMutex(),
383-
env.ColdWallet.BtcMinTransferFeeMultiplier,
384-
env.ColdWallet.RbtcMinTransferFeeMultiplier,
385-
env.ColdWallet.ForceTransferAfterSeconds,
386-
messaging.EventBus,
387-
rskRegistry.Wallet,
388-
signingHashFunction,
389-
),
373+
transferExcessToColdWalletUseCase: liquidity_provider.NewTransferExcessToColdWalletUseCase(
374+
lpRegistry.LiquidityProvider,
375+
lpRegistry.LiquidityProvider,
376+
lpRegistry.LiquidityProvider,
377+
databaseRegistry.LiquidityProviderRepository,
378+
lpRegistry.ColdWallet,
379+
btcRegistry.PaymentWallet,
380+
rskRegistry.Wallet,
381+
messaging.Rpc,
382+
mutexes.BtcWalletMutex(),
383+
mutexes.RskWalletMutex(),
384+
env.ColdWallet.BtcMinTransferFeeMultiplier,
385+
env.ColdWallet.RbtcMinTransferFeeMultiplier,
386+
env.ColdWallet.ForceTransferAfterSeconds,
387+
messaging.EventBus,
388+
rskRegistry.Wallet,
389+
signingHashFunction,
390+
),
390391
}
391392
}
392393

‎internal/entities/liquidity_provider/configuration.go‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"math/big"
77
"slices"
8-
"strconv"
98

109
"github.com/rsksmart/liquidity-provider-server/internal/entities"
1110
"github.com/rsksmart/liquidity-provider-server/internal/entities/utils"
@@ -105,21 +104,8 @@ type HashedCredentials struct {
105104
}
106105

107106
type StateConfiguration struct {
108-
LastBtcToColdWalletTransfer *int64 `json:"lastBtcToColdWalletTransfer" bson:"last_btc_to_cold_wallet_transfer"`
109-
LastRbtcToColdWalletTransfer *int64 `json:"lastRbtcToColdWalletTransfer" bson:"last_rbtc_to_cold_wallet_transfer"`
110-
}
111-
112-
// Implementing this so that we can log the state configuration in a readable format
113-
func (s StateConfiguration) String() string {
114-
btc := "nil"
115-
if s.LastBtcToColdWalletTransfer != nil {
116-
btc = strconv.FormatInt(*s.LastBtcToColdWalletTransfer, 10)
117-
}
118-
rbtc := "nil"
119-
if s.LastRbtcToColdWalletTransfer != nil {
120-
rbtc = strconv.FormatInt(*s.LastRbtcToColdWalletTransfer, 10)
121-
}
122-
return fmt.Sprintf("{LastBtcToColdWalletTransfer:%s LastRbtcToColdWalletTransfer:%s}", btc, rbtc)
107+
LastBtcToColdWalletTransfer int64 `json:"lastBtcToColdWalletTransfer" bson:"last_btc_to_cold_wallet_transfer"`
108+
LastRbtcToColdWalletTransfer int64 `json:"lastRbtcToColdWalletTransfer" bson:"last_rbtc_to_cold_wallet_transfer"`
123109
}
124110

125111
type ConfigurationType interface {

‎internal/entities/liquidity_provider/liquidity_provider.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ type LiquidityProvider interface {
7272
BtcAddress() string
7373
SignQuote(quoteHash string) (string, error)
7474
GeneralConfiguration(ctx context.Context) GeneralConfiguration
75-
StateConfiguration(ctx context.Context) StateConfiguration
75+
StateConfiguration(ctx context.Context) (StateConfiguration, error)
7676
GetSigner() entities.Signer
7777
}
7878

‎internal/usecases/liquidity_provider/initialize_state_configuration.go‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package liquidity_provider
22

33
import (
44
"context"
5+
"errors"
56
"time"
67

78
"github.com/rsksmart/liquidity-provider-server/internal/entities"
@@ -11,48 +12,44 @@ import (
1112
)
1213

1314
type InitializeStateConfigurationUseCase struct {
15+
provider liquidity_provider.LiquidityProvider
1416
lpRepository liquidity_provider.LiquidityProviderRepository
1517
signer entities.Signer
1618
hashFunc entities.HashFunction
1719
}
1820

1921
func NewInitializeStateConfigurationUseCase(
22+
provider liquidity_provider.LiquidityProvider,
2023
lpRepository liquidity_provider.LiquidityProviderRepository,
2124
signer entities.Signer,
2225
hashFunc entities.HashFunction,
2326
) *InitializeStateConfigurationUseCase {
2427
return &InitializeStateConfigurationUseCase{
28+
provider: provider,
2529
lpRepository: lpRepository,
2630
signer: signer,
2731
hashFunc: hashFunc,
2832
}
2933
}
3034

3135
func (useCase *InitializeStateConfigurationUseCase) Run(ctx context.Context) error {
32-
signedStateConfig, err := useCase.lpRepository.GetStateConfiguration(ctx)
33-
if err != nil {
36+
stateConfig, err := useCase.provider.StateConfiguration(ctx)
37+
if err != nil && !errors.Is(err, liquidity_provider.ConfigurationNotFoundError) {
3438
return usecases.WrapUseCaseError(usecases.InitializeStateConfigurationId, err)
3539
}
3640

37-
var stateConfig liquidity_provider.StateConfiguration
38-
if signedStateConfig != nil {
39-
stateConfig = signedStateConfig.Value
40-
}
41-
4241
modified := false
4342
now := time.Now().UTC().Unix()
4443

45-
// BTC field
46-
if stateConfig.LastBtcToColdWalletTransfer == nil {
44+
if stateConfig.LastBtcToColdWalletTransfer == 0 {
4745
log.Info("Initializing LastBtcToColdWalletTransfer with current timestamp")
48-
stateConfig.LastBtcToColdWalletTransfer = &now
46+
stateConfig.LastBtcToColdWalletTransfer = now
4947
modified = true
5048
}
5149

52-
// RBTC field
53-
if stateConfig.LastRbtcToColdWalletTransfer == nil {
50+
if stateConfig.LastRbtcToColdWalletTransfer == 0 {
5451
log.Info("Initializing LastRbtcToColdWalletTransfer with current timestamp")
55-
stateConfig.LastRbtcToColdWalletTransfer = &now
52+
stateConfig.LastRbtcToColdWalletTransfer = now
5653
modified = true
5754
}
5855

0 commit comments

Comments
 (0)