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

feat: add solana monitoring data #5277

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

marcellorigotti
Copy link
Contributor

Pull Request

Closes: PRO-1658

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have written sufficient tests.
  • I have written and tested required migrations.
  • I have updated documentation where appropriate.

Summary

Add solana monitoring data to monitoring rpc:

  • Available nonces/Unavailable nonces
  • Broadcaster.incomingKeyAndBroadcastId() -> This is missing for all the chains
    • Plus the signature of the transaction for solana -> we need to check it didn't revert
  • Solana aggKey
  • SolanaBroadcaster.currentOnChainKey -> Sol funds held here

Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 10.25641% with 35 lines in your changes missing coverage. Please review.

Project coverage is 70%. Comparing base (bd4cfa1) to head (f58a07e).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
state-chain/runtime/src/lib.rs 0% 0 Missing and 29 partials ⚠️
state-chain/custom-rpc/src/lib.rs 0% 4 Missing ⚠️
state-chain/runtime/src/monitoring_apis.rs 67% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##            main   #5277    +/-   ##
======================================
- Coverage     70%     70%    -0%     
======================================
  Files        487     487            
  Lines      86796   86697    -99     
  Branches   86796   86697    -99     
======================================
- Hits       60807   60608   -199     
- Misses     22655   22720    +65     
- Partials    3334    3369    +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@dandanlen dandanlen left a comment

Choose a reason for hiding this comment

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

Looks fine to me, i'd just like to understand better how this will be used. Let's chat tomorrow.

@marcellorigotti
Copy link
Contributor Author

Looks fine to me, i'd just like to understand better how this will be used. Let's chat tomorrow.

These changes are also breaking, bringing the same problems as #5279 does, correct?

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