fix(gov): Add value decoder for attribute key "sender" #1397
Conversation
WalkthroughThe changes suggest an enhancement in the governance and monitoring capabilities of a blockchain system. A method for converting account addresses from strings has been added to the governance precompile logic. Additionally, the configuration for the end-to-end test application now enables Prometheus metrics and telemetry with a memory sink for metrics. On the Ethereum integration side, new methods have been introduced to access the backend and networking stack of the execution layer, indicating an increased focus on interfacing with Ethereum-like blockchain components. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- cosmos/precompile/governance/governance.go (2 hunks)
- e2e/testapp/polard/cmd/commands.go (3 hunks)
- eth/eth.go (1 hunks)
- eth/polar/backend.go (1 hunks)
Additional comments: 7
eth/eth.go (2)
186-188: The addition of the
Backend()method to theExecutionLayerstruct is straightforward and provides access to the underlyingPolarisobject. This seems to be a utility function for internal use and does not appear to introduce any immediate issues.190-192: Similarly, the
Stack()method provides access to theNetworkingStackobject. This method is also straightforward and does not introduce any issues. It's important to ensure that theNetworkingStackobject is intended to be exposed in this manner.eth/polar/backend.go (1)
- 224-226: The
APIBackend()method has been added to thePolarisstruct to expose theapiBackendfield. This change is consistent with the pattern of providing accessor methods for internal fields and does not seem to introduce any issues. However, it's important to ensure that exposing this field is safe and intended, as it could potentially be used in a way that affects the system's behavior.e2e/testapp/polard/cmd/commands.go (2)
65-65: Enabling Prometheus in the
initCometBFTConfigfunction is a common practice for monitoring and observability purposes. This change aligns with the PR objectives to enhance monitoring capabilities.103-105: The changes in the
initAppConfigfunction to enable telemetry and set the metrics sink to "mem" are also in line with the PR's objectives to improve observability. It's important to ensure that these settings are appropriate for the test application's environment and that the "mem" sink is a suitable choice for the intended use case.cosmos/precompile/governance/governance.go (2)
- 569-574: The new method
ConvertAccAddressFromStringcorrectly uses thecosmlib.EthAddressFromStringfunction to convert a Cosmos account address from a string to an Ethereumcommon.Address. This is consistent with the PR's objective to decode the "sender" attribute key.Ensure that the
attributeValuepassed to this method is always a valid Cosmos account address string to prevent potential errors during the conversion process.
- 85-85: The integration of
ConvertAccAddressFromStringinto theCustomValueDecodersmethod is done correctly. It is mapped to thesdk.AttributeKeySender, which aligns with the PR's objective to handle the conversion of the "sender" attribute key.Ensure that the rest of the codebase is updated to use this new decoder where necessary, and that it is tested thoroughly to handle all expected input formats.
|
Closing in favor of #1396 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/precompile/governance/governance_test.go (1 hunks)
Additional comments: 1
cosmos/precompile/governance/governance_test.go (1)
- 110-110: The change in the expected length of custom value decoders from 2 to 3 is noted. This is likely due to the addition of a new decoder for the "sender" attribute key as mentioned in the PR objectives. Ensure that the new decoder is correctly implemented and tested.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1397 +/- ##
==========================================
- Coverage 48.25% 48.23% -0.02%
==========================================
Files 84 84
Lines 4965 4969 +4
==========================================
+ Hits 2396 2397 +1
- Misses 2392 2395 +3
Partials 177 177
|
Summary by CodeRabbit
New Features
Refactor