Payer Report Manager indexing #852
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
## Walkthrough
This change introduces support for indexing the "PayerReportManager" Ethereum contract. It adds configuration fields, contract wrappers, log storer logic, and validation for the new contract. The SettlementChain indexer is updated to initialize, track, and process events from the PayerReportManager, with associated tests and configuration updates.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------|
| pkg/config/options.go, pkg/testutils/config.go | Added new config fields for PayerRegistryAddress, PayerReportManagerAddress, MaxChainDisconnectTime. |
| pkg/config/validation.go | Added validation logic for new SettlementChain config fields. |
| pkg/indexer/settlement_chain/contracts/payer_report_manager.go | Added contract wrapper for PayerReportManager, including initialization and accessor methods. |
| pkg/indexer/settlement_chain/contracts/payer_report_manager_storer.go | Added log storer for PayerReportManager events, with event parsing and error handling. |
| pkg/indexer/settlement_chain/contracts/payer_report_manager_storer_test.go | Added tests for PayerReportManagerStorer covering error and event handling. |
| pkg/indexer/settlement_chain/settlement_chain.go | Updated SettlementChain to manage, initialize, and start indexing for PayerReportManager. |
| pkg/indexer/app_chain/app_chain.go, pkg/blockchain/rpcLogStreamer.go | Updated logger usage and RpcLogStreamer signature (removed chainID parameter, logger context change). |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Config as Config
participant SC as SettlementChain
participant PRM as PayerReportManager
participant RLS as RpcLogStreamer
participant DB as Database
Config->>SC: Provide PayerReportManagerAddress, MaxChainDisconnectTime
SC->>PRM: Initialize contract wrapper with address
SC->>RLS: Register PRM contract address, topics, disconnect time
SC->>PRM: Retrieve latest block number
SC->>RLS: Start log streaming for PRM
RLS->>PRM: Emit event logs
PRM->>DB: Store processed event logsAssessment against linked issues
Possibly related PRs
Suggested reviewers
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Add settlement chain indexing for Payer Registry and Payer Report Manager contracts with 300s maximum chain disconnect timeAdds settlement chain monitoring capabilities to the indexer by creating new contract interfaces for • New settlement chain configuration fields in options.go for contract addresses and a 300s maximum disconnect time 📍Where to StartStart with the Macroscope summarized 99549bb. |
Add PayerReportManager contract indexing to SettlementChain with configuration support and remove chainID parameter from RpcLogStreamerThis PR adds support for indexing the PayerReportManager contract within the SettlementChain implementation. The changes include: • Add new 📍Where to StartStart with the Macroscope summarized a51441d. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (4)
pkg/indexer/settlement_chain/settlement_chain.go (1)
20-23: Track the TODO for rpc log streamer changes.The TODO comment indicates this is temporary code. Consider tracking this in an issue to ensure it's addressed when the rpc log streamer changes are introduced.
Do you want me to open a new issue to track this TODO?
pkg/indexer/settlement_chain/contracts/payer_report_manager.go (1)
18-21: Use plural form for slice variable name.The variable contains multiple topics, so it should use the plural form for clarity.
Apply this diff:
-var payerReportManagerTopic = []string{ +var payerReportManagerTopics = []string{ "PayerReportSubmitted", "PayerReportSubsetSettled", }Also update the usage at line 113:
- topics := make([]common.Hash, len(payerReportManagerTopic)) - for i, topic := range payerReportManagerTopic { + topics := make([]common.Hash, len(payerReportManagerTopics)) + for i, topic := range payerReportManagerTopics {pkg/indexer/settlement_chain/contracts/payer_registry.go (2)
18-23: Fix variable naming for consistency.The variable name should be plural since it contains multiple topic names.
-var payerRegistryTopic = []string{ +var payerRegistryTopics = []string{Don't forget to update the reference in line 116:
- for i, topic := range payerRegistryTopic { + for i, topic := range payerRegistryTopics {
83-93: Fix receiver naming to avoid confusion.The receiver name
prconflicts with the imported package aliasprfrom line 9, which can cause confusion.-func (pr *PayerRegistry) Address() common.Address { - return pr.address +func (p *PayerRegistry) Address() common.Address { + return p.address } -func (pr *PayerRegistry) Topics() []common.Hash { - return pr.topics +func (p *PayerRegistry) Topics() []common.Hash { + return p.topics } -func (pr *PayerRegistry) Logger() *zap.Logger { - return pr.logger +func (p *PayerRegistry) Logger() *zap.Logger { + return p.logger }
🛑 Comments failed to post (2)
pkg/indexer/settlement_chain/settlement_chain.go (2)
65-85:
⚠️ Potential issueIncomplete resource cleanup on initialization failure.
When
NewPayerReportManagerfails, thepayerRegistryresource is not cleaned up. This could lead to resource leaks.Apply this diff to ensure proper cleanup:
payerReportManager, err := contracts.NewPayerReportManager( ctxwc, client, querier, chainLogger, common.HexToAddress(cfg.PayerReportManagerAddress), cfg.ChainID, ) if err != nil { cancel() client.Close() + // Add any necessary cleanup for payerRegistry if it has resources return nil, err }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.payerReportManager, err := contracts.NewPayerReportManager( ctxwc, client, querier, chainLogger, common.HexToAddress(cfg.PayerReportManagerAddress), cfg.ChainID, ) if err != nil { cancel() client.Close() // Add any necessary cleanup for payerRegistry if it has resources return nil, err }🤖 Prompt for AI Agents
In pkg/indexer/settlement_chain/settlement_chain.go between lines 65 and 85, when NewPayerReportManager returns an error, the payerRegistry resource is not being closed, causing a potential resource leak. To fix this, add a call to payerRegistry.Close() before cancel() and client.Close() in the error handling block after NewPayerReportManager fails, ensuring all allocated resources are properly cleaned up on initialization failure.
120-129:
⚠️ Potential issueMissing payerReportManager field assignment.
The
payerReportManagerfield is not assigned in the struct initialization, which will cause nil pointer issues when accessed.Apply this diff to fix the missing assignment:
return &SettlementChain{ - ctx: ctxwc, - cancel: cancel, - client: client, - log: chainLogger, - streamer: streamer, - chainID: cfg.ChainID, - payerRegistry: payerRegistry, + ctx: ctxwc, + cancel: cancel, + client: client, + log: chainLogger, + streamer: streamer, + chainID: cfg.ChainID, + payerRegistry: payerRegistry, + payerReportManager: payerReportManager, }, nil🤖 Prompt for AI Agents
In pkg/indexer/settlement_chain/settlement_chain.go around lines 120 to 129, the SettlementChain struct initialization is missing the assignment for the payerReportManager field. To fix this, add the payerReportManager field with the appropriate variable or value during the struct initialization to prevent nil pointer dereference errors when this field is accessed later.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/blockchain/rpcLogStreamer.go(1 hunks)pkg/indexer/app_chain/app_chain.go(1 hunks)pkg/indexer/settlement_chain/settlement_chain.go(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/indexer/app_chain/app_chain.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/indexer/settlement_chain/settlement_chain.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test (Node)
- GitHub Check: Upgrade Tests
fbac
force-pushed
the
05-27-payer_report_manager_indexing
branch
from
8162d95 to
a51441d
Compare
mkysel
reviewed
May 27, 2025
| IDENTITY_UPDATE_BROADCAST_ADDRESS = "0xDCbC334a97c6a8DBe7d673bd52fA56718708BC9C" | ||
| PARAMETER_REGISTRY_ADDRESS = "0x7FFc148AF5f00C56D78Bce732fC79a08007eC8be" | ||
| PAYER_REGISTRY_ADDRESS = "0x6ADCc064469C3b69ED6Fa4DbeFA21490FD200D7c" | ||
| PAYER_REPORT_MANAGER_ADDRESS = "0x2aA7BC557FF0b9B55FFD82274706DD2aD37E687B" |
Collaborator
There was a problem hiding this comment.
do we already have this in local.env?
mkysel
approved these changes
May 27, 2025
This was referenced May 28, 2025
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #851
Summary by CodeRabbit