feat: offchain bot implementation for ggp alerts #240
Conversation
musitdev
left a comment
musitdev
left a comment
There was a problem hiding this comment.
Mostly, I don't see why you:
- add the _ on all these files
- you use the indexer to get the balance
The indexer usage is to get withdraw events to verify after each even, but you decide to pool which is fine so I don't see why you use it for the balance.
For the alert, you should start a health check entry point that the sre team can plug with pager duty. This is a basic example .
| ), | ||
| ident_str!("abort_bridge_transfer").to_owned(), | ||
| vec![], | ||
| vec![bcs::to_bytes(&bridge_transfer_id).unwrap()], |
There was a problem hiding this comment.
The variable is used, why you precede with an _ ? And more generally, why you do that for all the other var?
|
|
||
| /// Query current coin balance (AptosCoin) for an owner address via GraphQL indexer. | ||
| /// Tries `current_coin_balances` first; if unavailable, falls back to latest row from `coin_balances`. | ||
| pub async fn get_latest_apt_balance(&self, owner: &str) -> anyhow::Result<Option<u64>> { |
There was a problem hiding this comment.
Why you don't use the RPC to get the balance, it's easier and doesn't add a dependency to the indexer?
You've implemented the call.
| impl RestClient { | ||
| pub fn new(base: String) -> Self { Self { base, http: reqwest::Client::new() } } | ||
|
|
||
| pub async fn get_apt_balance(&self, account: &str) -> anyhow::Result<u64> { |
| indexer: &IndexerClient, | ||
| sinks: &[Arc<dyn AlertSink>], | ||
| ) -> anyhow::Result<()> { | ||
| let balance = indexer.get_latest_apt_balance(ggp_address).await?.unwrap_or(0); |
There was a problem hiding this comment.
There's a view function to get the ggp balance:
public fun get_balance<CoinType>(): u64 acquires GovernedGasPool
I think it's the best way to get the ggp balance. If the ggp treasury change, this call will still return the right balance.
| @@ -0,0 +1,65 @@ | |||
| use serde::Deserialize; | |||
| pub num_validators_hint: u64, | ||
| } | ||
|
|
||
| pub fn compute_dynamic_low_threshold(p: Option<ThresholdParams>) -> u64 { |
There was a problem hiding this comment.
Same in the move contract, explain the threshold function ins the comment. We need to understand why the threshold is calculated like that.
Description
GGP balance alert bot that monitors the GGP balance via the Movement indexer and sends alerts when the balance falls below a dynamically computed threshold.
Running the bot requires some env variables to be set:
Required env vars
Optional env vars
Run
Type of Change
Which Components or Systems Does This Change Impact?
How Has This Been Tested?
All unit tests in
threshold.rspassing.Key Areas to Review
crates/ggp-balance-alert-bot/src/threshold.rscrates/ggp-balance-alert-bot/src/sources/rest.rscrates/ggp-balance-alert-bot/src/sources/indexer.rsChecklist