chore(api): use func to return txData #2884
Conversation
WalkthroughThe changes introduce a new feature in the API by refactoring the Changes
Poem
Warning Review ran into problems🔥 ProblemsError running LanguageTool: LanguageTool error: Unknown error 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 (
|
|
https://github.com/Mergifyio backport rama/release-v20 |
✅ Backports have been created
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
api/ethermint/evm/v1/msg.go (2)
15-21: Excellent change to prevent pointer reuse!The modification of
supportedTxsto return new instances of transaction types is a great improvement. It effectively addresses the concern of pointer reuse and potential race conditions, aligning perfectly with the PR objectives.Consider slightly rewording the comment for clarity:
- // Use a function to return a new pointer and avoid - // possible reuse or racing conditions when using the same pointer + // Use functions to return new pointers, avoiding + // potential reuse or race conditions that could occur with shared pointers
42-46: Good implementation of the new approachThe changes to the
GetSignersfunction correctly utilize the updatedsupportedTxsmap. This ensures that a new instance of the transaction data is created each time, preventing any potential issues with shared state.Consider a small optimization to reduce the number of map lookups:
-txDataFn, found := supportedTxs[msgEthTx.Data.TypeUrl] -if !found { - return nil, fmt.Errorf("invalid TypeUrl %s", msgEthTx.Data.TypeUrl) -} -txData := txDataFn() +if txDataFn, found := supportedTxs[msgEthTx.Data.TypeUrl]; found { + txData := txDataFn() +} else { + return nil, fmt.Errorf("invalid TypeUrl %s", msgEthTx.Data.TypeUrl) +}This change combines the lookup and error checking, slightly improving performance and readability.
CHANGELOG.md (1)
Line range hint
110-1037: LGTM: CHANGELOG.md is well-maintained and provides a comprehensive history.The CHANGELOG.md file maintains a consistent format throughout its history, providing a detailed record of changes for each version from v15.0.0 back to v0.1.0. This comprehensive changelog is valuable for users and developers to understand the evolution of the project.
Consider adding a note at the beginning of the file mentioning whether the project follows Semantic Versioning (SemVer). This information helps users understand the versioning scheme used in the project.
🧰 Tools
🪛 Markdownlint
61-61: Expected: 120; Actual: 133
Line length(MD013, line-length)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- api/ethermint/evm/v1/msg.go (2 hunks)
🔇 Additional comments (3)
api/ethermint/evm/v1/msg.go (1)
Line range hint
1-58: Summary: Excellent improvements to transaction handlingThe changes in this file significantly enhance the safety and correctness of Ethereum transaction handling. By modifying
supportedTxsto return new instances of transaction types, the code now prevents potential issues related to shared pointers and race conditions.Key improvements:
- Enhanced safety by avoiding pointer reuse
- Clear comments explaining the rationale behind the changes
- Consistent implementation throughout the file
These changes align perfectly with the PR objectives and represent a valuable improvement to the codebase. Great work!
CHANGELOG.md (2)
64-64: LGTM: Unreleased section is well-structured and informative.The Unreleased section is organized clearly with appropriate subsections for State Machine Breaking changes, Improvements, and Bug Fixes. Each entry is properly formatted with the affected module/area and includes links to the corresponding pull requests. This structure makes it easy for users to understand the upcoming changes in the next release.
Line range hint
66-108: LGTM: v15.0.0 section is comprehensive and well-formatted.The v15.0.0 section provides a detailed overview of the changes in this release. It includes API Breaking, State Machine Breaking, Improvements, and Bug Fixes subsections, each with well-documented changes and links to the corresponding pull requests. The format is consistent with the Unreleased section, maintaining readability throughout the changelog.
🧰 Tools
🪛 Markdownlint
61-61: Expected: 120; Actual: 133
Line length(MD013, line-length)
* chore(api): use func to return txData * add changelog entry * update comment (cherry picked from commit 30ef902) # Conflicts: # CHANGELOG.md
* chore(api): use func to return txData (#2884) * chore(api): use func to return txData * add changelog entry * update comment (cherry picked from commit 30ef902) # Conflicts: # CHANGELOG.md * update chlog --------- Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com> Co-authored-by: tom <tomasguerraalda@hotmail.com>
Description
This change assures were not re-using the same pointer on calls to the
GetSignerfunctions.More context here
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
Reviewers Checklist
All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.
I have...
Unreleasedsection inCHANGELOG.mdSummary by CodeRabbit
New Features
GetSingers, enhancing the API's ability to retrieve transaction data.Improvements