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: extensible main chain follower #581

Closed

Conversation

justinfrevert
Copy link

@justinfrevert justinfrevert commented Mar 17, 2025

Description

Any given partnerchain who wishes to perform custom queries, and also send custom data through mainchain follower has a difficult time progressing because of the small surface area of the main chain follower API, and the amount of dependencies in the code. Such teams may fork the entire main-chain-follower to update a few lines, but for Midnight, this is a blocker. It's an unmaintainable amount of code required for relatively small changes, and mostly consists of code that is required to sustain a general partner-chain(e.g. db-sync queries for block data), which is not the responsibility of a partner-chain implementer.

There may be an approach to provide custom queries without forking any code, but if there is such a path, it's entirely unclear. I'll argue that an approach enabled by this PR is much clearer, and provides a nice repo structure to any implementing partnerchain. E.g. for extending db_model, the repo can include their own db_model, import the partner-chains module, and can base their "extension" solely on their own query.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages.
  • The size limit of 400 LOC isn't needlessly exceeded
  • The PR refers to a JIRA ticket (if one exists)
  • New tests are added if needed and existing tests are updated.
  • Relevant logging and metrics added
  • Any changes are noted in the changelog.md for affected crate
  • Self-reviewed the diff

@@ -607,7 +607,7 @@ pub(crate) async fn index_exists(pool: &Pool<Postgres>, index_name: &str) -> boo
/// Sums all transfers between genesis and the first block that is produced with the feature on.
/// Used by `get_native_token_transfers` (NativeTokenDataSourceImpl).
#[cfg(feature = "native-token")]
pub(crate) async fn get_total_native_tokens_transfered(
pub async fn get_total_native_tokens_transfered(
Copy link
Contributor

Choose a reason for hiding this comment

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

This functionality can be accessed through the already public API: NativeTokenManagementDataSource::get_total_native_token_transfer

@@ -641,7 +641,7 @@ AND block.block_no <= $4;
/// On devnet postgres it takes around 5ms to execute when querying 1000 blocks.
/// Used by `get_native_token_transfers` (NativeTokenDataSourceImpl).
#[cfg(feature = "native-token")]
pub(crate) async fn get_native_token_transfers(
pub async fn get_native_token_transfers(
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be already covered by NativeTokenManagementDataSource::get_total_native_token_transfer already

@@ -347,7 +347,7 @@ WHERE
}

#[cfg(any(feature = "block-source", feature = "native-token"))]
pub(crate) async fn get_latest_block_info(
pub async fn get_latest_block_info(
Copy link
Contributor

@AmbientTea AmbientTea Mar 17, 2025

Choose a reason for hiding this comment

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

This is already provided by BlockDataSourceImpl::get_latest_block_info, which is public but wasn't really intended to be. We could arrange to officially expose it

Copy link
Author

Choose a reason for hiding this comment

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

We should - it seems more useful to expose it than to stretch BlockDataSourceImpl into a place where it doesn't make sense.

@justinfrevert
Copy link
Author

Closing in preference of #594.

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