Skip to content

Commit 40b9c47

Browse files
AndresQuijanoAndresQuijano
committed
fix: feedback comments
1 parent abd1d73 commit 40b9c47

26 files changed

+852
-451
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ tools: download
1818
pip3 install pre-commit && pre-commit install
1919
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(shell go env GOPATH)/bin v1.63.4
2020
go install github.com/ethereum/go-ethereum/cmd/abigen@eb00f1694c9265f6909c19995a535eef246dcf1e # v1.14.13
21-
go install github.com/vektra/mockery/v2@v2.51.1 # ensures mockery version 2.51.1 is installed
21+
go install github.com/vektra/mockery/v2@v2.53.1 # ensures mockery version 2.53.1 is installed
2222

2323
download:
2424
go mod download

‎docs/Environment.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ These are the environment variables required by the liquidity provider server (L
6969
| `ECLIPSE_BTC_MAX_MS_WAIT_FOR_BLOCK` | Number of milliseconds that the LPS will wait for our BTC node to catch up with the external datasources before assuming the node is eclipsed | `1000` | No |
7070
| `ECLIPSE_BTC_WAIT_POLLING_MS_INTERVAL` | Polling interval in ms to check our BTC node when is out of sync with the external data sources | `500` | No |
7171
| `ECLIPSE_ALERT_COOLDOWN_SECONDS` | Number of seconds after an eclipsed node detection that the watcher will wait to start checking again | `3600` | No |
72-
| `BTC_MIN_TRANSFER_FEE_MULTIPLIER` | Minimum multiplier for BTC transfers to cold wallet to be considered economical (transfer amount must be >= fee * multiplier) | `5` | No |
73-
| `RBTC_MIN_TRANSFER_FEE_MULTIPLIER` | Minimum multiplier for RBTC transfers to cold wallet to be considered economical (transfer amount must be >= fee * multiplier) | `100` | No |
72+
| `BTC_MIN_TRANSFER_FEE_MULTIPLIER` | It is a plain number meaning the minimum multiplier for BTC transfers to cold wallet to be considered economical (transfer amount must be >= fee * multiplier) | `5` | No |
73+
| `RBTC_MIN_TRANSFER_FEE_MULTIPLIER` | It is a plain number meaning the minimum multiplier for RBTC transfers to cold wallet to be considered economical (transfer amount must be >= fee * multiplier) | `100` | No |
7474
| `COLD_WALLET_FORCE_TRANSFER_AFTER_SECONDS` | Number of seconds after which excess liquidity will be transferred to cold wallet even if below threshold | `1209600` (2 weeks) | No |
7575

7676
## AWS variables

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (repo *lpMongoRepository) UpsertCredentials(ctx context.Context, credential
105105
func (repo *lpMongoRepository) GetStateConfiguration(ctx context.Context) (*entities.Signed[liquidity_provider.StateConfiguration], error) {
106106
dbCtx, cancel := context.WithTimeout(ctx, repo.conn.timeout)
107107
defer cancel()
108-
return getConfiguration[liquidity_provider.StateConfiguration](dbCtx, repo, stateConfigId, false)
108+
return getConfigurationVerbose[liquidity_provider.StateConfiguration](dbCtx, repo, stateConfigId)
109109
}
110110

111111
func (repo *lpMongoRepository) UpsertStateConfiguration(ctx context.Context, configuration entities.Signed[liquidity_provider.StateConfiguration]) error {
@@ -115,7 +115,7 @@ func (repo *lpMongoRepository) UpsertStateConfiguration(ctx context.Context, con
115115
Signed: configuration,
116116
Name: stateConfigId,
117117
}
118-
return upsertConfiguration(dbCtx, repo, configToStore, false)
118+
return upsertConfigurationVerbose(dbCtx, repo, configToStore)
119119
}
120120

121121
func upsertConfigurationVerbose[C liquidity_provider.ConfigurationType](

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,6 @@ func TestLpMongoRepository_GetStateConfiguration(t *testing.T) {
352352
repo := mongo.NewLiquidityProviderRepository(mongo.NewConnection(client, time.Duration(1)))
353353
collection.On("FindOne", mock.Anything, filter).
354354
Return(mongoDb.NewSingleResultFromDocument(testStateConfig, nil, nil)).Once()
355-
defer test.AssertNoLog(t)()
356355
result, err := repo.GetStateConfiguration(context.Background())
357356
require.NoError(t, err)
358357
assert.Equal(t, testStateConfig, result)
@@ -390,7 +389,6 @@ func TestLpMongoRepository_UpsertStateConfiguration(t *testing.T) {
390389
Name: configName,
391390
}, options.Replace().SetUpsert(true)).
392391
Return(nil, nil).Once()
393-
defer test.AssertNoLog(t)()
394392
err := repo.UpsertStateConfiguration(context.Background(), *testStateConfig)
395393
require.NoError(t, err)
396394
})

‎internal/adapters/dataproviders/liquidity_provider.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,17 @@ 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 {
209+
configuration, err := liquidity_provider.ValidateConfiguration(lp.signer, crypto.Keccak256, func() (*entities.Signed[liquidity_provider.StateConfiguration], error) {
210+
return lp.lpRepository.GetStateConfiguration(ctx)
211+
})
212+
if err != nil {
213+
lp.logConfigError("state", err)
214+
return liquidity_provider.StateConfiguration{}
215+
}
216+
return configuration.Value
217+
}
218+
208219
func (lp *LocalLiquidityProvider) GetSigner() entities.Signer {
209220
return lp.signer
210221
}

‎internal/adapters/dataproviders/liquidity_provider_test.go‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,3 +715,155 @@ func getPegoutConfigurationMock() *entities.Signed[liquidity_provider.PegoutConf
715715
Hash: "40e2e3f42928a80814f19d897bb3da4119bff12e15cdb60125d9c2f82c590ea3",
716716
}
717717
}
718+
719+
//nolint:funlen
720+
func TestLocalLiquidityProvider_StateConfiguration(t *testing.T) {
721+
account := test.OpenWalletForTest(t, "state-configuration")
722+
wallet := rootstock.NewRskWalletImpl(&rootstock.RskClient{}, account, 31, time.Duration(1))
723+
lpRepository := new(mocks.LiquidityProviderRepositoryMock)
724+
725+
t.Run("Return signed state configuration from db", func(t *testing.T) {
726+
// Create properly signed config using the test wallet
727+
btcTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)
728+
rbtcTime := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC)
729+
stateConfigValue := liquidity_provider.StateConfiguration{
730+
LastBtcToColdWalletTransfer: &btcTime,
731+
LastRbtcToColdWalletTransfer: &rbtcTime,
732+
}
733+
734+
signedConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
735+
require.NoError(t, err)
736+
737+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&signedConfig, nil).Once()
738+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
739+
result := lp.StateConfiguration(context.Background())
740+
741+
assert.Equal(t, stateConfigValue, result)
742+
assert.NotNil(t, result.LastBtcToColdWalletTransfer)
743+
assert.NotNil(t, result.LastRbtcToColdWalletTransfer)
744+
})
745+
746+
t.Run("Return empty state configuration when db doesn't have configuration", func(t *testing.T) {
747+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(nil, nil).Once()
748+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, nil, nil, blockchain.RskContracts{})
749+
750+
defer test.AssertLogContains(t, "Custom state configuration not found. Using default configuration.")()
751+
752+
config := lp.StateConfiguration(context.Background())
753+
754+
// StateConfiguration has no defaults - returns empty struct
755+
assert.Nil(t, config.LastBtcToColdWalletTransfer)
756+
assert.Nil(t, config.LastRbtcToColdWalletTransfer)
757+
})
758+
759+
t.Run("Return empty state configuration on db read error", func(t *testing.T) {
760+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(nil, errors.New("database error")).Once()
761+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, nil, nil, blockchain.RskContracts{})
762+
763+
defer test.AssertLogContains(t, "Error getting state configuration, using default configuration. Error: database error")()
764+
config := lp.StateConfiguration(context.Background())
765+
766+
assert.Nil(t, config.LastBtcToColdWalletTransfer)
767+
assert.Nil(t, config.LastRbtcToColdWalletTransfer)
768+
})
769+
770+
t.Run("Return empty state configuration when db configuration is tampered", func(t *testing.T) {
771+
btcTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)
772+
rbtcTime := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC)
773+
stateConfigValue := liquidity_provider.StateConfiguration{
774+
LastBtcToColdWalletTransfer: &btcTime,
775+
LastRbtcToColdWalletTransfer: &rbtcTime,
776+
}
777+
778+
tamperedConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
779+
require.NoError(t, err)
780+
781+
// Tamper with the data after signing
782+
newTime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
783+
tamperedConfig.Value.LastBtcToColdWalletTransfer = &newTime
784+
785+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&tamperedConfig, nil).Once()
786+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
787+
788+
defer test.AssertLogContains(t, "Tampered state configuration. Using default configuration.")()
789+
result := lp.StateConfiguration(context.Background())
790+
791+
assert.Nil(t, result.LastBtcToColdWalletTransfer)
792+
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
793+
})
794+
795+
t.Run("Return empty state configuration when db configuration doesn't have valid signature", func(t *testing.T) {
796+
btcTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)
797+
rbtcTime := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC)
798+
stateConfigValue := liquidity_provider.StateConfiguration{
799+
LastBtcToColdWalletTransfer: &btcTime,
800+
LastRbtcToColdWalletTransfer: &rbtcTime,
801+
}
802+
803+
// Create config with correct hash but wrong signature
804+
signedConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
805+
require.NoError(t, err)
806+
// Use a different but valid signature format (from different wallet/data)
807+
signedConfig.Signature = "94530cf2d078ce7e44b4ce1d63a0cf7a225f07d4414f4dcf132f097fd027c08c7252b012ffff6855400fbc96939662904b22ce0b7a010bcb0b7a2c7db9dc26b700"
808+
809+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&signedConfig, nil).Once()
810+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
811+
812+
defer test.AssertLogContains(t, "Invalid state configuration signature. Using default configuration.")()
813+
814+
result := lp.StateConfiguration(context.Background())
815+
816+
assert.Nil(t, result.LastBtcToColdWalletTransfer)
817+
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
818+
})
819+
820+
t.Run("Return empty state configuration when integrity check fails", func(t *testing.T) {
821+
btcTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)
822+
rbtcTime := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC)
823+
stateConfigValue := liquidity_provider.StateConfiguration{
824+
LastBtcToColdWalletTransfer: &btcTime,
825+
LastRbtcToColdWalletTransfer: &rbtcTime,
826+
}
827+
828+
integrityFailConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
829+
require.NoError(t, err)
830+
831+
// Change hash to make integrity check fail
832+
integrityFailConfig.Hash = "83cb825a5f8dcf1bdd3cd33effffda7a34ed8b0d80a39445049ddc9c06ecb1a9"
833+
834+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&integrityFailConfig, nil).Once()
835+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
836+
837+
defer test.AssertLogContains(t, "Tampered state configuration. Using default configuration.")()
838+
839+
result := lp.StateConfiguration(context.Background())
840+
841+
assert.Nil(t, result.LastBtcToColdWalletTransfer)
842+
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
843+
})
844+
845+
t.Run("Return empty state configuration when there is an unexpected error", func(t *testing.T) {
846+
btcTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)
847+
rbtcTime := time.Date(2024, 1, 20, 14, 45, 0, 0, time.UTC)
848+
stateConfigValue := liquidity_provider.StateConfiguration{
849+
LastBtcToColdWalletTransfer: &btcTime,
850+
LastRbtcToColdWalletTransfer: &rbtcTime,
851+
}
852+
853+
integrityFailConfig, err := usecases.SignConfiguration(usecases.InitializeStateConfigurationId, wallet, crypto.Keccak256, stateConfigValue)
854+
require.NoError(t, err)
855+
856+
// Use invalid hash format that will fail hex decoding
857+
integrityFailConfig.Hash = "an-invalid-hash"
858+
859+
lpRepository.On("GetStateConfiguration", test.AnyCtx).Return(&integrityFailConfig, nil).Once()
860+
lp := dataproviders.NewLocalLiquidityProvider(nil, nil, lpRepository, blockchain.Rpc{}, wallet, nil, blockchain.RskContracts{})
861+
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())
865+
866+
assert.Nil(t, result.LastBtcToColdWalletTransfer)
867+
assert.Nil(t, result.LastRbtcToColdWalletTransfer)
868+
})
869+
}

‎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{})
260259
}
261260

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

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ func TestAssetReportWatcher_Start(t *testing.T) {
130130
watcher := monitoring.NewAssetReportWatcher(appMetrics, assetsReportUseCase, ticker)
131131

132132
// Verify initial metric values are zero
133-
initialRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, "rbtc", "total")
134-
initialBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "btc", "allocation_available")
133+
initialRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelTotal)
134+
initialBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelBtc, monitoring.MetricLabelAllocationAvailable)
135135
assert.InDelta(t, 0.0, initialRbtcTotal, 0.0001)
136136
assert.InDelta(t, 0.0, initialBtcAvailable, 0.0001)
137137

@@ -146,15 +146,15 @@ func TestAssetReportWatcher_Start(t *testing.T) {
146146
time.Sleep(50 * time.Millisecond)
147147

148148
// Verify metrics were updated with expected values
149-
finalRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, "rbtc", "total")
150-
finalBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "btc", "allocation_available")
149+
finalRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelTotal)
150+
finalBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelBtc, monitoring.MetricLabelAllocationAvailable)
151151
assert.InDelta(t, 9.5, finalRbtcTotal, 0.0001)
152152
assert.InDelta(t, 4.5, finalBtcAvailable, 0.0001)
153153

154154
// Verify labels are correct
155-
rbtcLabels := getAssetGaugeLabels(appMetrics.AssetsMetrics, "rbtc", "total")
156-
assert.Equal(t, "rbtc", rbtcLabels["currency"])
157-
assert.Equal(t, "total", rbtcLabels["type"])
155+
rbtcLabels := getAssetGaugeLabels(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelTotal)
156+
assert.Equal(t, monitoring.MetricLabelRbtc, rbtcLabels["currency"])
157+
assert.Equal(t, monitoring.MetricLabelTotal, rbtcLabels["type"])
158158

159159
// Properly shutdown and wait for completion
160160
closeChannel := make(chan bool, 1)
@@ -182,8 +182,8 @@ func TestAssetReportWatcher_Start(t *testing.T) {
182182
watcher := monitoring.NewAssetReportWatcher(appMetrics, assetsReportUseCase, ticker)
183183

184184
// Verify initial metric values are zero
185-
initialRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, "rbtc", "total")
186-
initialBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "btc", "allocation_available")
185+
initialRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelTotal)
186+
initialBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelBtc, monitoring.MetricLabelAllocationAvailable)
187187
assert.InDelta(t, 0.0, initialRbtcTotal, 0.0001)
188188
assert.InDelta(t, 0.0, initialBtcAvailable, 0.0001)
189189

@@ -198,8 +198,8 @@ func TestAssetReportWatcher_Start(t *testing.T) {
198198
time.Sleep(50 * time.Millisecond)
199199

200200
// Verify metrics remain unchanged (zero) due to error
201-
finalRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, "rbtc", "total")
202-
finalBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "btc", "allocation_available")
201+
finalRbtcTotal := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelTotal)
202+
finalBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelBtc, monitoring.MetricLabelAllocationAvailable)
203203
assert.InDelta(t, 0.0, finalRbtcTotal, 0.0001)
204204
assert.InDelta(t, 0.0, finalBtcAvailable, 0.0001)
205205

@@ -296,8 +296,8 @@ func TestAssetReportWatcher_Start(t *testing.T) {
296296
time.Sleep(50 * time.Millisecond)
297297

298298
// Verify first update
299-
firstRbtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "rbtc", "allocation_available")
300-
firstBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "btc", "allocation_available")
299+
firstRbtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelAllocationAvailable)
300+
firstBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelBtc, monitoring.MetricLabelAllocationAvailable)
301301
assert.InDelta(t, 3.0, firstRbtcAvailable, 0.0001)
302302
assert.InDelta(t, 2.0, firstBtcAvailable, 0.0001)
303303

@@ -306,8 +306,8 @@ func TestAssetReportWatcher_Start(t *testing.T) {
306306
time.Sleep(50 * time.Millisecond)
307307

308308
// Verify second update (metrics should reflect new values)
309-
secondRbtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "rbtc", "allocation_available")
310-
secondBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, "btc", "allocation_available")
309+
secondRbtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelRbtc, monitoring.MetricLabelAllocationAvailable)
310+
secondBtcAvailable := getAssetGaugeValue(appMetrics.AssetsMetrics, monitoring.MetricLabelBtc, monitoring.MetricLabelAllocationAvailable)
311311
assert.InDelta(t, 4.0, secondRbtcAvailable, 0.0001)
312312
assert.InDelta(t, 3.0, secondBtcAvailable, 0.0001)
313313

0 commit comments

Comments
 (0)