Skip to content

Commit 9710449

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

File tree

17 files changed

+309
-341
lines changed

17 files changed

+309
-341
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/adapters/entrypoints/watcher/monitoring/cold_wallet_metrics_watcher_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,11 +400,11 @@ func TestColdWalletMetricsWatcher_Start_MultipleEvents(t *testing.T) {
400400
finalBtcGauge := getGaugeVecValue(appMetrics.ColdWalletLastAmountMetric, monitoring.MetricLabelBtc)
401401

402402
// RBTC gauge should be either 0.5 (last threshold) or 1.5 (time_forcing) depending on processing order
403-
assert.True(t, finalRbtcGauge == 0.5 || finalRbtcGauge == 1.5,
403+
assert.True(t, finalRbtcGauge == 0.5 || finalRbtcGauge == 1.5,
404404
"RBTC gauge should be 0.5 or 1.5, got %.2f", finalRbtcGauge)
405-
405+
406406
// BTC gauge should be either 2.5 (last threshold) or 3.0 (time_forcing) depending on processing order
407-
assert.True(t, finalBtcGauge == 2.5 || finalBtcGauge == 3.0,
407+
assert.True(t, finalBtcGauge == 2.5 || finalBtcGauge == 3.0,
408408
"BTC gauge should be 2.5 or 3.0, got %.2f", finalBtcGauge)
409409

410410
watcher.Shutdown(make(chan bool, 1))

‎internal/adapters/entrypoints/watcher/monitoring/metric_labels.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ const (
1414

1515
// Asset metric type labels
1616
const (
17-
MetricLabelTotal = "total"
18-
MetricLabelLocationRskWallet = "location_rsk_wallet"
19-
MetricLabelLocationBtcWallet = "location_btc_wallet"
20-
MetricLabelLocationLbc = "location_lbc"
21-
MetricLabelLocationFederation = "location_federation"
22-
MetricLabelAllocationReservedForUsers = "allocation_reserved_for_users"
23-
MetricLabelAllocationWaitingRefund = "allocation_waiting_refund"
24-
MetricLabelAllocationAvailable = "allocation_available"
17+
MetricLabelTotal = "total"
18+
MetricLabelLocationRskWallet = "location_rsk_wallet"
19+
MetricLabelLocationBtcWallet = "location_btc_wallet"
20+
MetricLabelLocationLbc = "location_lbc"
21+
MetricLabelLocationFederation = "location_federation"
22+
MetricLabelAllocationReservedForUsers = "allocation_reserved_for_users"
23+
MetricLabelAllocationWaitingRefund = "allocation_waiting_refund"
24+
MetricLabelAllocationAvailable = "allocation_available"
2525
)

0 commit comments

Comments
 (0)