Skip to content
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

Merged
merged 4 commits into from
Oct 18, 2024
Merged

Conversation

longbowlu
Copy link
Collaborator

@longbowlu longbowlu commented Oct 16, 2024

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:

Copy link

vercel bot commented Oct 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 18, 2024 8:09pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Oct 18, 2024 8:09pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Oct 18, 2024 8:09pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Oct 18, 2024 8:09pm

@longbowlu longbowlu force-pushed the bridge-auth-agg-metrics branch from b5c2c54 to b89985a Compare October 16, 2024 16:55
@longbowlu longbowlu closed this Oct 16, 2024
@longbowlu longbowlu force-pushed the add-watchdog-to-bridge-node branch from 9a538a1 to b89985a Compare October 16, 2024 23:15
@longbowlu longbowlu deleted the add-watchdog-to-bridge-node branch October 16, 2024 23:17
@longbowlu longbowlu restored the add-watchdog-to-bridge-node branch October 16, 2024 23:17
@longbowlu longbowlu reopened this Oct 17, 2024
@longbowlu longbowlu changed the title add watchdog to bridge node [bridge] add watchdog to bridge node Oct 17, 2024
@longbowlu longbowlu marked this pull request as ready for review October 17, 2024 01:25
@longbowlu longbowlu marked this pull request as draft October 17, 2024 07:09
@longbowlu longbowlu marked this pull request as ready for review October 17, 2024 07:09
Base automatically changed from bridge-auth-agg-metrics to main October 18, 2024 03:07
watchdog_config: Some(WatchdogConfig {
total_supplies: BTreeMap::from_iter(vec![(
"eth".to_string(),
"0xd0e89b2af5e4910726fbcd8b8dd37bb79b29e5f83f7491bca830e94f7f226d29::eth::ETH"
Copy link
Contributor

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?

Copy link
Collaborator Author

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

Copy link
Contributor

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;
Copy link
Contributor

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

Copy link
Contributor

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.

Copy link
Collaborator Author

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)
Copy link
Contributor

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@Bridgerz Bridgerz left a 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.

@longbowlu longbowlu enabled auto-merge (squash) October 18, 2024 20:09
@longbowlu longbowlu merged commit 54bc3f0 into main Oct 18, 2024
50 checks passed
@longbowlu longbowlu deleted the add-watchdog-to-bridge-node branch October 18, 2024 20:32
longbowlu added a commit that referenced this pull request Oct 18, 2024
## 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:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants