pendulum-chain / spacewalk

Apache License 2.0
35 stars 7 forks source link

Support liquidated vaults in metrics #559

Closed gianfra-t closed 1 month ago

gianfra-t commented 1 month ago

Issue: #548.

Description of the issue.

Right now, when we start the client pointing to a liquidated vault, the vault will never be saved into the VaultIdManager created on initiation (see here).

The issue is that many of the metrics code relies on iterating this map to obtain the metrics from on-chain data (eg.: here or here).

Proposed workaround

Add a new field to VaultData to keep track of a liquidated vault, and add the vault even if the status is Liquidated when the systems starts.

Modifying the get methods like get_entries to filter by default non-liquidated vaults allows to maintain compatibility, while in metrics we fetch all vaults on the VaultIdManager.

Testing

To verify that the missing exports, I ran a new vault against the testnet, but momentarily patching the code such that it was not added to VaultIdManager. When exploring the metrics, I could see that most of them are missing.

Extra metric

Added a Liquidated metric that displays if the vault is liquidated.

# HELP liquidated Boolean reporting if the vault is currently liquidated
# TYPE liquidated gauge
liquidated{currency="XCM(0)_Stellar(USDC:GAKNDFRRWA3RPWNLTI3G4EBSD3RGNZZOY5WKWYMQ6CQTG3KIEKPYWAYC)"} 0

This takes the liquidated status from VaultData, this is currently NOT updated and is only updated upon restart. I believe the vault will not be set back to regular status since the recover extrinsic does not emit. Also, we are not polling to check the status of it from the state which means that a liquidated vault will stay in this state (in the client) until restart.

Furthermore, we will not detect if the vault is liquidated while the client is running. Although it was observed that this leads to a restart of the client in production.

gianfra-t commented 1 month ago

Yes I tested it with the local spacewalk testnet, registering one of the example vaults but commenting here (to avoid simulating a liquidation). With that, none of the vaults metrics were exported which makes sense since there is nothing to iterate form VaultIdManager.

With this new change the metrics should show, all of them. We can later decide to filter some of them based on the liquidated value.

I would've liked to test the restart that happens when the client runs a liquidated vault, but I have no way of reproducing it.

gianfra-t commented 1 month ago

That's great @ebma ! Thanks for testing, I kinda ignored Amplitude since I couldn't connect to polkadot.js and then forgot to check there. I'll merge this then!