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

SyncFinalityProviderStatus updates status for non-existent Finality Providers #73

Open
gusin13 opened this issue Sep 26, 2024 · 3 comments

Comments

@gusin13
Copy link
Collaborator

gusin13 commented Sep 26, 2024

Bug Description

SyncFinalityProviderStatus incorrectly updating the status of Finality Providers (FPs) that don't actually exist on the consumer chain.

vp, err := app.cc.QueryFinalityProviderVotingPower(fp.BtcPk, latestBlock.Height)

Current Behavior

  1. SyncFinalityProviderStatus queries voting power for all locally stored FPs without checking their existence on the chain.
  2. Some consumer implementations of QueryFinalityProviderVotingPower return a response (with zero power) even for non-existent FPs.
  3. This causes SyncFinalityProviderStatus to update statuses of FPs that aren't actually on the chain.

Expected Behavior

SyncFinalityProviderStatus should only update statuses for FPs that actually exist on the consumer chain.

Root Cause

  1. Lack of existence check in SyncFinalityProviderStatus before querying power.
  2. Inconsistent implementation of QueryFinalityProviderVotingPower across consumers.

Notes

The Babylon contract (used in babylon-sdk) currently returns a zero power response instead of an error for non-existent FPs:

root@2bb4c3b05f8d:/ibcsim-bcd# bcd q wasm contract-state smart bbnc1nc5tatafv6eyq7llkr2gv50ff9e22mnf70qgjlv737ktmt4eswrqgn0kq0 '{"finality_provider_info":{"btc_pk_hex": "018bd93a74d49a28dac942f90716fc715a6ca9dce6fcb527211c735f693fddc8"}}'
data:
  btc_pk_hex: 018bd93a74d49a28dac942f90716fc715a6ca9dce6fcb527211c735f693fddc8
  power: 0

this causes the SyncFinalityProviderStatus to mark the FP as REGISTERED even if it doesn't exist on the consumer chain

This problem doesn't occur for Babylon finality provider as Babylon grpc query returns err if FP doesn't exist

@gusin13
Copy link
Collaborator Author

gusin13 commented Sep 26, 2024

Fixed the query in babylon-contract
babylonlabs-io/babylon-contract#71

@RafilxTenfen
Copy link
Contributor

Hey @gusin13, the idea of SyncFinalityProviderStatus is to mark it as registered if the FP exists on chain
Should we use a query that verifies that the FP exists on the consumer chain?

@gusin13
Copy link
Collaborator Author

gusin13 commented Oct 18, 2024

@RafilxTenfen yes correct, we can first check if the FP exists on the consumer chain and then query VP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants