-
Notifications
You must be signed in to change notification settings - Fork 11.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bridge] add watchdog to bridge node #19878
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
b5c2c54
to
b89985a
Compare
9a538a1
to
b89985a
Compare
8d3ba77
to
4f24daa
Compare
watchdog_config: Some(WatchdogConfig { | ||
total_supplies: BTreeMap::from_iter(vec![( | ||
"eth".to_string(), | ||
"0xd0e89b2af5e4910726fbcd8b8dd37bb79b29e5f83f7491bca830e94f7f226d29::eth::ETH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just for mainnet correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just an example if you expand up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha
// 2. i64::MAX is 9_223_372_036_854_775_807, with 8 decimal places is | ||
// 92_233_720_368. We likely won't see any balance higher than this | ||
// in the next 12 months. | ||
let balance = (balance / self.ten_zeros).as_u64() as i64; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only works for Eth decimals tho right? We'll need to update this when we include tokens with decimals other than 18 ya?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if we leave it as is and add a new token with a different decimal is the reported balance for that token will be a little off until we update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will have to update this before the new token is added
} | ||
|
||
fn interval(&self) -> Duration { | ||
Duration::from_secs(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this match eth_bridge_status duration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #19878 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We'll need to make sure we circle back to this when we support the next token.
## Description 1. run watchdogs on bridge nodes. 2. merges `sui-bridge-watchdog` to `sui-bridge` crate, so there is no circular dependencies ## Test plan unit tests, will deploy locally and test --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
Description
sui-bridge-watchdog
tosui-bridge
crate, so there is no circular dependenciesTest plan
unit tests, will deploy locally and test
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.