(DAL) Remove config id reference #2290
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve modifications to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (12)
node/pkg/reporter/app_test.go (1)
84-84: LGTM: Condition updated to uselen(symbols).The change in the condition to compare against
len(symbols)instead oflen(configs)is consistent with the earlier renaming. The logic remains the same, maintaining the existing functionality while aligning with the new symbol-based approach.Consider extracting
len(symbols)to a variable at the beginning of the function for improved readability and to avoid repeated calls tolen(). For example:func TestWsDataHandling(t *testing.T) { ctx := context.Background() app := New() conn, tmpConfig, symbols, err := mockDalWsServer(ctx) if err != nil { t.Fatalf("error mocking dal ws server: %v", err) } + symbolsCount := len(symbols) // ... rest of the function ... - if submissionDataCount == len(symbols) { + if submissionDataCount == symbolsCount { return } // ... rest of the function ... - if submissionDataCount != len(symbols) { + if submissionDataCount != symbolsCount { t.Fatal("not all submission data received from websocket") } }Also applies to: 89-89
node/pkg/reporter/main_test.go (2)
59-62: LGTM! Consider future removal ofconfigId.The addition of the
Symbolfield to theSubmissionDatastruct is consistent with the PR objectives of moving away from config ID references. This change aligns well with the shift towards symbol-based representation.For future improvements, consider removing the
configIdparameter and its usage within this function, as it may no longer be necessary given the shift towards symbol-based operations.
65-65: LGTM! Consider future removal oftmpConfig.The changes to the
mockDalWsServerfunction effectively implement the shift from config-based to symbol-based operations:
- The return type change from
[]types.Configto[]stringremoves the dependency on the Config type.- The introduction of the
symbolsvariable and its usage inNewCollectorandHubSetupaligns with the new symbol-based approach.These modifications successfully address the PR objectives of removing config ID references and reducing dependencies.
For future improvements, consider removing the
tmpConfigvariable and its usage within this function, as it may no longer be necessary given the shift towards symbol-based operations. This would further streamline the code and reduce any remaining dependencies on the Config type.Also applies to: 84-84, 89-89, 95-95, 112-112
node/pkg/dal/tests/collector_test.go (1)
Line range hint
1-114: Summary: Consistent changes, consider broader impact.The modifications in this file consistently update the assertions from
collector.Symbolstocollector.FeedHashes, aligning with the PR objectives to remove config ID references. These changes reflect a shift in the collector's implementation from using symbols to feed hashes.While these test updates are straightforward, they likely indicate more extensive changes in the collector's core implementation and potentially other related components.
Consider reviewing the following areas to ensure consistency and completeness of this refactoring:
- The core implementation of the
Collectorstruct and its methods in thepkg/dal/collectorpackage.- Any other components or packages that interact with the collector and might be affected by this change (e.g., hub, websocket handlers).
- Documentation and comments that might need updating to reflect the shift from symbols to feed hashes.
- Any configuration files or deployment scripts that might reference the old symbol-based structure.
This broader review will help ensure that the refactoring is complete and consistent across the entire codebase.
node/pkg/dal/tests/main_test.go (2)
71-71: LGTM! Consider adding a comment for clarity.The addition of the
Symbolfield to theSubmissionDatastruct aligns with the PR objective of removing config ID references. This change enhances the flexibility of the data structure.Consider adding a brief comment explaining the purpose of the
Symbolfield for better code documentation:return &aggregator.SubmissionData{ + // Symbol represents the unique identifier for the aggregation configuration Symbol: symbol, GlobalAggregate: sampleGlobalAggregate, Proof: proof, }, nil
Line range hint
1-168: Overall, the changes look good and align with the PR objectives.The modifications successfully remove config ID references and simplify the DAL setup process by using symbols instead of config objects. This refactoring appears to be part of a larger effort to transition from config IDs to symbols throughout the codebase.
To ensure the completeness of this refactoring:
- Verify that all related components (e.g.,
Collector,Hub,aggregator) have been updated to work with symbols instead of config IDs.- Update any relevant documentation to reflect these changes.
- Consider adding or updating unit tests to cover the new symbol-based functionality.
As this change affects the core data structures and initialization process, it's crucial to ensure that all dependent systems and interfaces are updated accordingly. Consider creating a migration plan if this change affects existing data or APIs.
node/pkg/dal/apiv2/controller.go (1)
175-177: LGTM! Consider usingmakewith a predefined capacity.The change from
s.hub.Configstos.hub.Symbolsaligns well with the PR objective of removing config ID references. This modification simplifies the data structure and focuses directly on symbols, which is more appropriate for the DAL.For a minor optimization, consider using
makewith a predefined capacity:- result := make([]string, 0, len(s.hub.Symbols)) + result := make([]string, 0, len(s.hub.Symbols))This change can slightly improve performance by allocating the correct amount of memory upfront, reducing potential reallocations.
node/pkg/dal/hub/hub.go (2)
Line range hint
80-84: Updatesubscriptionshandling to match new typeAfter changing
Clientsto usemap[string]struct{}, ensuresubscriptionsinHandleSubscriptionis handled correctly.Apply this diff to update
subscriptions:func (h *Hub) HandleSubscription(ctx context.Context, client *websocket.Conn, msg Subscription, id int32) { h.mu.Lock() defer h.mu.Unlock() - subscriptions, ok := h.Clients[client] + subscriptions, ok := h.Clients[client] + var subscriptions map[string]struct{} if !ok { - subscriptions = map[string]any{} + subscriptions = make(map[string]struct{}) } valid := []string{} for _, param := range msg.Params { symbol := strings.TrimPrefix(param, "submission@") if _, ok := h.Symbols[symbol]; !ok { continue } - subscriptions[symbol] = struct{}{} + subscriptions[symbol] = struct{}{} valid = append(valid, param) } h.Clients[client] = subscriptions
61-62: Consider limiting the number of goroutines spawnedIn
Start, a goroutine is started for each symbol inh.Symbols. If the number of symbols is large, this could lead to resource exhaustion. Consider using a worker pool or limiting the number of goroutines.node/pkg/dal/collector/collector.go (3)
95-97: InitializeLatestTimestampswith the length ofsymbolsIn the constructor, while you're initializing
OutgoingStreamandFeedHasheswith the length ofsymbols, consider initializingLatestTimestampswith the same capacity to optimize memory allocation.Apply this diff to initialize
LatestTimestampswith predefined capacity:- LatestTimestamps: make(map[string]time.Time), + LatestTimestamps: make(map[string]time.Time, len(symbols)),
163-163: Optimize slice capacity inGetAllLatestDataWhen initializing the
resultslice inGetAllLatestData, use the length ofc.LatestDatato match the actual number of elements you expect to collect.Apply this diff to adjust the capacity:
-result := make([]dalcommon.OutgoingSubmissionData, 0, len(c.FeedHashes)) +result := make([]dalcommon.OutgoingSubmissionData, 0, len(c.LatestData))
219-219: Correct typo in log messageThe log message contains a typo: "recieved" should be "received". Correcting this improves code quality and professionalism.
Apply this diff to fix the typo:
-log.Debug().Str("Player", "DalCollector").Str("Symbol", data.Symbol).Msg("old data recieved") +log.Debug().Str("Player", "DalCollector").Str("Symbol", data.Symbol).Msg("old data received")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
- node/pkg/aggregator/types.go (1 hunks)
- node/pkg/aggregator/utils.go (1 hunks)
- node/pkg/dal/apiv2/controller.go (1 hunks)
- node/pkg/dal/app.go (3 hunks)
- node/pkg/dal/collector/collector.go (8 hunks)
- node/pkg/dal/hub/hub.go (4 hunks)
- node/pkg/dal/tests/collector_test.go (2 hunks)
- node/pkg/dal/tests/main_test.go (2 hunks)
- node/pkg/reporter/app_test.go (2 hunks)
- node/pkg/reporter/main_test.go (3 hunks)
🔇 Additional comments (11)
node/pkg/aggregator/utils.go (1)
Line range hint
31-54: Consider addressing remainingconfigIdreferences for consistency with PR objectives.While the change to
PublishGlobalAggregateAndProofis good, I noticed that other functions in this file still useconfigId:
getLatestRoundIdgetProofFromPgsgetLatestGlobalAggregateFromPgsThese functions might need to be updated to align with the PR objective of removing config ID references. Consider refactoring these functions to use the
nameorSymbolinstead ofconfigId, if applicable.Additionally, if any of these functions are related to the Node Admin API, they might need to be modified or removed to meet the objective of removing the Node Admin API dependency.
To help with this refactoring, let's check for all occurrences of
configIdin the codebase:#!/bin/bash # Description: Find all occurrences of configId in Go files echo "Occurrences of configId in Go files:" rg --type go 'configId'node/pkg/reporter/app_test.go (2)
79-84: LGTM: Loop iteration updated to usesymbols.The change in the loop to iterate over
symbolsinstead ofconfigsis consistent with the earlier renaming. The logic inside the loop remains unchanged, maintaining the existing functionality while aligning with the new symbol-based approach.
Line range hint
36-89: Summary: Successful transition from config-based to symbol-based approach.The changes in this file consistently replace
configswithsymbols, aligning with the PR objective of removing config ID references. This transition maintains the existing functionality while moving towards a symbol-based approach for data handling. The modifications are applied consistently throughout theTestWsDataHandlingfunction, affecting the function call, loop iteration, and condition checks.These changes contribute to the overall goal of streamlining the Data Access Layer (DAL) by reducing its dependencies on configuration IDs. The test now focuses on symbols, which likely represents a more flexible and maintainable approach for handling data in the reporter package.
node/pkg/dal/tests/collector_test.go (2)
54-54: LGTM. Verify collector implementation.The assertion has been consistently updated to check
collector.FeedHashesinstead ofcollector.Symbols, maintaining alignment with the changes in theTestCollectorStartAndStopfunction.To ensure that these test changes accurately reflect the collector's implementation, let's verify the collector's code:
#!/bin/bash # Description: Verify the collector's implementation to ensure it uses FeedHashes instead of Symbols. # Test 1: Check the collector's struct definition echo "Checking the collector's struct definition:" rg --type go 'type Collector struct' pkg/dal/collector # Test 2: Search for methods or functions that use FeedHashes echo "Searching for methods or functions that use FeedHashes:" rg --type go 'FeedHashes' pkg/dal/collector # Test 3: Ensure there are no remaining references to Symbols in the collector echo "Checking for any remaining references to Symbols in the collector:" rg --type go 'Symbols' pkg/dal/collector
34-34: LGTM. Verify related changes.The assertion has been updated to check
collector.FeedHashesinstead ofcollector.Symbols, which aligns with the PR objectives of removing the config ID reference. This change is consistent with the shift in focus described in the AI-generated summary.To ensure consistency across the codebase, let's verify related changes:
✅ Verification successful
Verification Successful: No Remaining References to
SymbolsThe search confirmed that there are no remaining references to
collector.Symbolsin the collector package or related tests. This indicates that the replacement withcollector.FeedHasheshas been successfully completed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to `Symbols` in the collector package and related tests. # Test 1: Search for remaining references to `Symbols` in the collector package echo "Searching for remaining references to 'Symbols' in the collector package:" rg --type go 'collector\.Symbols' pkg/dal/collector # Test 2: Search for any other test files that might need similar updates echo "Searching for other test files that might need updates:" rg --type go 'collector\.Symbols' pkg/dal/testsLength of output: 679
Script:
#!/bin/bash # Description: Locate the Collector struct definition in the codebase. rg --type go 'type Collector struct' .Length of output: 102
Script:
#!/bin/bash # Description: Check for any remaining references to `Symbols` in the collector package and related tests. # Test 1: Search for remaining references to `Symbols` in the collector package echo "Searching for remaining references to 'Symbols' in the collector package:" rg --type go 'collector\.Symbols' node/pkg/dal/collector # Test 2: Search for any other test files that might need similar updates echo "Searching for other test files that might need updates:" rg --type go 'collector\.Symbols' node/pkg/dal/testsLength of output: 397
node/pkg/dal/tests/main_test.go (2)
104-104: LGTM! Good simplification of the setup process.Replacing the
configsslice with asymbolsslice aligns well with the PR objective of removing config ID references. This change simplifies the setup process by using symbol names directly instead of config objects.
109-109: LGTM! Verify Collector and Hub implementations.The changes to
NewCollectorandHubSetupfunction calls, usingsymbolsinstead ofconfigs, are consistent with the PR objectives and previous modifications. This ensures that both theCollectorandHubare initialized with symbol names rather than config objects.To ensure these changes are properly implemented throughout the codebase, please run the following verification script:
This script will help verify that the
CollectorandHubimplementations have been updated to handle symbols correctly and that there are no remaining references to config IDs in these components.Also applies to: 116-116
✅ Verification successful
Verified: Collector and Hub implementations handle symbols correctly.
The
NewCollectorandHubSetupfunctions have been successfully updated to usesymbolsinstead ofconfigs. Additionally, there are no remaining references toconfigIdorConfigIDin thecollectorandhubpackages, ensuring a complete transition.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that Collector and Hub implementations handle symbols correctly # Test: Check Collector implementation echo "Checking Collector implementation:" ast-grep --lang go --pattern $'func NewCollector(ctx context.Context, symbols []string) (*Collector, error) { $$$ }' # Test: Check Hub implementation echo "Checking Hub implementation:" ast-grep --lang go --pattern $'func HubSetup(ctx context.Context, symbols []string) *Hub { $$$ }' # Test: Verify no remaining references to config IDs in Collector and Hub echo "Checking for remaining config ID references:" rg --type go 'configId|ConfigID' node/pkg/dal/collector node/pkg/dal/hubLength of output: 6761
node/pkg/aggregator/types.go (1)
36-36: LGTM: Addition of Symbol field to SubmissionData structThe addition of the
Symbolfield to theSubmissionDatastruct is straightforward and aligns with the AI-generated summary mentioning symbol handling changes.To ensure consistency across the codebase, please verify all usages of
SubmissionData:Additionally, could you clarify how this change relates to the PR objectives of removing config ID reference and Node Admin API dependency? It seems this addition might be part of a larger refactoring effort.
node/pkg/dal/apiv2/controller.go (1)
Line range hint
1-324: Verify the impact of removing config ID and Node Admin API dependencies.The changes in this file align well with the PR objectives. However, it's crucial to ensure that removing the config ID references and Node Admin API dependencies doesn't have unintended consequences in other parts of the codebase.
Please run the following script to check for any remaining references to config IDs or the Node Admin API in the rest of the codebase:
This script will help identify any remaining instances that might need attention in other files.
node/pkg/dal/collector/collector.go (2)
34-36: Ensure consistent update of map key types fromint32tostringthroughout the codebaseThe keys for
OutgoingStream,FeedHashes, andLatestTimestampshave been changed fromint32tostring. Please verify that all references to these maps elsewhere in the codebase have been updated to usestringkeys to prevent any runtime issues.Run the following script to find potential usages with outdated key types:
#!/bin/bash # Description: Search for map accesses using `int32` keys. # Search for patterns where maps are accessed using int32 keys. rg --type go 'OutgoingStream\[[^\]]*int32[^\]]*\]' rg --type go 'FeedHashes\[[^\]]*int32[^\]]*\]' rg --type go 'LatestTimestamps\[[^\]]*int32[^\]]*\]'
53-53: Verify that all calls toNewCollectorare updated to match the new signatureThe
NewCollectorfunction now accepts a[]stringofsymbolsinstead of[]Config. Ensure that all invocations of this function have been updated accordingly to prevent compilation errors.Run the following script to locate all calls to
NewCollector:✅ Verification successful
Generate a more precise search for all
NewCollectorinvocations.
Update the script to correctly search for
.gofiles and locate allNewCollectorinvocations.
All calls to
NewCollectorhave been successfully updated to match the new signature.
- Verified invocations in:
node/pkg/dal/tests/main_test.gonode/pkg/dal/app.gonode/pkg/reporter/main_test.go🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of `NewCollector` and check their arguments. rg --type go 'NewCollector\(' -A 2Length of output: 156
Script:
# #!/bin/bash # Description: Locate all calls to `NewCollector` and display their arguments. fd --type go | xargs rg 'NewCollector\s*\(' -A 2Length of output: 1111
Script:
# #!/bin/bash # Description: Locate all calls to `NewCollector` and display their arguments. fd --extension go | xargs rg 'NewCollector\s*\(' -A 2Length of output: 946
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- node/pkg/dal/app.go (4 hunks)
- node/pkg/dal/collector/collector.go (8 hunks)
- node/pkg/dal/hub/hub.go (7 hunks)
- node/pkg/error/sentinel.go (1 hunks)
🔇 Additional comments (21)
node/pkg/dal/app.go (5)
5-5: LGTM: Import changes are appropriate.The addition of the "fmt" package and the modification of the "errorsentinel" import align with the new functionality introduced in this file.
Also applies to: 15-15
34-37: LGTM: Proper handling of the CHAIN environment variable.The code correctly checks for the presence of the
CHAINenvironment variable and logs an error message if it's not set. This change aligns with the new approach of using chain-specific configurations.
40-44: LGTM: Proper implementation of symbol fetching.The code correctly calls the new
fetchSymbolsfunction and handles potential errors appropriately. The error logging before returning ensures that issues are properly tracked.
46-53: LGTM: Collector and hub initialization updated correctly.The initialization of the collector and hub has been properly updated to use the fetched symbols instead of configurations. This change aligns with the new approach of working with symbols directly.
Line range hint
1-86: Overall changes align well with PR objectives.The modifications in this file successfully achieve the goals of removing the config ID reference and the Node Admin API dependency. The transition to using symbols directly simplifies the data flow and reduces dependencies. Error handling has been improved throughout the file, enhancing the robustness of the application.
Some suggestions for further improvement have been provided in previous comments, particularly regarding the
fetchSymbolsfunction and the hardcoded base URL. Addressing these points will further enhance the code quality and maintainability.node/pkg/dal/hub/hub.go (9)
23-24: LGTM: Improved type safety and idiomatic Go usageThe changes to the
SymbolsandClientsfields in theHubstruct are improvements:
- Using
map[string]struct{}forSymbolsis more idiomatic in Go for representing sets.- The consistent use of
map[string]struct{}forClientssubscriptions improves type safety.These changes align with Go best practices and make the code more robust.
36-42: LGTM: SimplifiedHubSetupfunctionThe changes to the
HubSetupfunction are appropriate:
- Accepting a slice of strings simplifies the function signature and removes the dependency on the
Configtype.- The implementation correctly creates a set-like structure using
map[string]struct{}.These changes make the function more straightforward and align with the new
Hubstructure.
Line range hint
46-53: LGTM: Consistent updates toNewHubfunctionThe changes to the
NewHubfunction are appropriate and consistent with theHubstruct updates:
- The function now accepts
symbols map[string]struct{}, aligning with the newSymbolsfield type.- The
Clientsfield is correctly initialized withmap[string]struct{}for subscriptions, ensuring type safety.These changes maintain consistency throughout the codebase and improve type safety.
61-63: LGTM: Improved iteration and goroutine safetyThe changes in the
Startmethod are well-implemented:
- Iterating over
h.Symbolsis consistent with the updatedHubstruct.- Capturing the loop variable
symbolinsymprevents potential data races in the goroutine.The introduction of
symis particularly important for avoiding the common pitfall of sharing loop variables across goroutines. This change enhances the concurrency safety of the code.
Line range hint
75-85: LGTM: Consistent updates toHandleSubscriptionmethodThe changes in the
HandleSubscriptionmethod are appropriate and consistent:
- Initializing subscriptions as
map[string]struct{}aligns with the updatedClientsfield type.- The symbol validity check now correctly uses
h.Symbols, maintaining consistency with the newHubstructure.These changes ensure that the subscription handling remains consistent with the updated
Hubstruct and improve type safety.
117-117: LGTM: Consistent client subscription initializationThe change in the
addClientmethod is correct:
- Initializing the client's subscriptions as
map[string]struct{}is consistent with the updatedClientsfield type in theHubstruct.This change maintains consistency throughout the codebase and ensures type safety for client subscriptions.
147-147: LGTM: Improved parameter passing inbroadcastDataForSymbolThe change in the
broadcastDataForSymbolmethod is appropriate:
- Passing
symbolby value instead of by pointer tocastSubmissionDatais more efficient for small types like strings.This change aligns with Go best practices for parameter passing and is consistent with the updated
castSubmissionDatamethod signature.
151-158: LGTM: Improved parameter handling and consistency incastSubmissionDataThe changes in the
castSubmissionDatamethod are well-implemented:
- Accepting
symbol stringinstead ofsymbol *stringis more efficient for small types like strings.- The symbol check using
subscriptions[symbol]is consistent with the updatedClientsfield type.These changes improve performance, maintain consistency with the
Hubstruct updates, and enhance type safety.
Line range hint
1-200: Overall assessment: Well-implemented refactoringThe changes in this file represent a comprehensive refactoring from using
Configobjects to using symbols directly. Key improvements include:
- Consistent use of
map[string]struct{}for set-like data structures, improving type safety and adhering to Go idioms.- Simplified function signatures and method implementations, reducing complexity.
- Improved concurrency safety by capturing loop variables in goroutines.
- Consistent updates across all affected methods, maintaining the integrity of the
Hubstructure and its operations.These changes should result in a more maintainable, type-safe, and efficient codebase. The refactoring appears to be thorough and well-executed.
node/pkg/dal/collector/collector.go (6)
34-37: LGTM: Improved map key types for better readabilityThe change from
int32tostringkeys in theOutgoingStream,FeedHashes, andLatestTimestampsmaps is a good improvement. It makes the code more intuitive and aligns with the use of symbols (typically represented as strings) instead of config IDs.
53-53: LGTM: Simplified NewCollector function signatureThe change from
configs []Configtosymbols []stringin theNewCollectorfunction signature is a good simplification. It aligns with the PR objective of removing the config ID reference and focuses the function on the essential data needed for initialization.
Line range hint
163-169: LGTM: Updated GetAllLatestData methodThe changes in the
GetAllLatestDatamethod are consistent with the removal of theSymbolsfield from theCollectorstruct. The use oflen(c.FeedHashes)for the capacity of theresultslice is correct and maintains the efficiency of the previous implementation. The logic remains functionally equivalent while adapting to the new structure.
203-205: LGTM: Updated compareAndSwapLatestTimestamp methodThe change from
data.ConfigIDtodata.Symbolin thecompareAndSwapLatestTimestampmethod is consistent with the overall shift in the codebase. The logic of the method remains intact, ensuring that it continues to function correctly with the new symbol-based approach.
219-219: LGTM: Updated log message in processIncomingDataThe change from
data.ConfigIDtodata.Symbolin the log message of theprocessIncomingDatamethod is consistent with the overall changes in the codebase. The log message remains informative and useful for debugging purposes.
Line range hint
242-276: LGTM: Updated IncomingDataToOutgoingData method with improved error handlingThe changes in the
IncomingDataToOutgoingDatamethod are well-implemented and consistent with the overall shift from config IDs to symbols. The addition of theSymbolfield in theOutgoingSubmissionDatastruct provides more complete information.The new check for
FeedHashexistence (lines 243-246) addresses the concern raised in the past review comment about handling potential missingFeedHash. This addition improves the robustness of the code and uses a sentinel error, which is a good practice for error handling.node/pkg/error/sentinel.go (1)
214-216: LGTM! New error definitions align with PR objectives.The addition of these new error definitions (ErrDalFeedHashNotFound, ErrDalSymbolsNotFound, and ErrDalChainEnvNotFound) aligns well with the PR objectives of removing the config ID reference and Node Admin API dependency from the Data Access Layer (DAL). These new errors will help in handling specific scenarios that may arise due to the refactoring.
To ensure these new errors are being utilized in the codebase, let's run the following verification script:
This script will help us confirm that the new error definitions are being used appropriately in the codebase.
✅ Verification successful
All new DAL error definitions are properly utilized within the codebase.
The new error definitions (
ErrDalFeedHashNotFound,ErrDalSymbolsNotFound, andErrDalChainEnvNotFound) have been successfully used in the following files:
node/pkg/dal/collector/collector.gonode/pkg/dal/app.goThis confirms that the errors are appropriately integrated and serve their intended purpose.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of newly added DAL errors in the codebase. # Test: Search for the usage of new error variables echo "Searching for ErrDalFeedHashNotFound usage:" rg "ErrDalFeedHashNotFound" --type go echo "Searching for ErrDalSymbolsNotFound usage:" rg "ErrDalSymbolsNotFound" --type go echo "Searching for ErrDalChainEnvNotFound usage:" rg "ErrDalChainEnvNotFound" --type goLength of output: 1033
Description
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment