-
Notifications
You must be signed in to change notification settings - Fork 14
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
refactor(solidity): rename _channelIBC to _memo for addBridgeToken #948
Conversation
WalkthroughThe pull request introduces a systematic change across multiple Solidity and Go files, replacing the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
solidity/contracts/bridge/FxBridgeLogicETH.sol (1)
107-107
: LGTM! Function parameter and event argument renamed.The parameter has been renamed from
_channelIBC
to_memo
in both the function signature and event emission while maintaining the same type (bytes32).Consider adding a comment to document this parameter rename, similar to the existing comment for
state_fxOriginatedToken
:+ /** + * @dev Parameter renamed from _channelIBC to _memo after fxcore upgrade v3 + */ function addBridgeToken( address _tokenAddr, bytes32 _memo, bool _isOriginated ) public onlyOwner returns (bool) {Also applies to: 124-124
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
contract/ifx_bridge_logic.sol.go
(5 hunks)solidity/contracts/bridge/FxBridgeBase.sol
(1 hunks)solidity/contracts/bridge/FxBridgeLogic.sol
(2 hunks)solidity/contracts/bridge/FxBridgeLogicETH.sol
(2 hunks)solidity/contracts/interfaces/IFxBridgeLogic.sol
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (6)
solidity/contracts/interfaces/IFxBridgeLogic.sol (1)
42-42
: LGTM! Function parameter renamed.The function parameter has been renamed from
_channelIBC
to_memo
while maintaining the same type (bytes32).solidity/contracts/bridge/FxBridgeLogic.sol (1)
96-96
: LGTM! Function parameter and event argument renamed.The parameter has been renamed from
_channelIBC
to_memo
in both the function signature and event emission while maintaining the same type (bytes32).Also applies to: 113-113
contract/ifx_bridge_logic.sol.go (4)
72-72
: LGTM: ABI string updated correctly.The ABI string has been properly updated to reflect the parameter rename from
_channelIBC
to_memo
while maintaining the same type (bytes32
).
771-787
: LGTM: Function signatures updated consistently.The
AddBridgeToken
function signatures have been updated consistently across all implementations (Transactor, Session, and TransactorSession) to use the new parameter name_memo
while maintaining the same type and behavior.
Line range hint
1053-1118
: LGTM: Event definitions updated consistently.The
AddBridgeTokenEvent
related code has been updated consistently:
- Event struct field renamed from
ChannelIBC
toMemo
- Event signatures updated in all related functions
- Type maintained as
[32]byte
Line range hint
1-1
: Verify consistency with Solidity interface.Since this is an auto-generated binding file, let's verify that the changes match the Solidity interface modifications.
Run this script to check the parameter naming in the Solidity files:
✅ Verification successful
Parameter naming is correctly implemented for chain-specific interfaces
The parameter naming is intentionally different between chain implementations:
- ETH chain interface uses
_memo
- BSC chain interface uses
_channelIBC
- Go bindings correctly reflect the interface definition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify parameter rename consistency between Solidity and Go files # Check Solidity files for the new parameter name echo "Checking Solidity files for _memo parameter..." rg --type solidity "function addBridgeToken.*_memo.*bytes32" -A 2 # Check for any remaining instances of old parameter name echo -e "\nChecking for any remaining instances of old parameter name..." rg --type solidity "function addBridgeToken.*_channelIBC.*bytes32" -A 2Length of output: 449
Script:
#!/bin/bash # Find Solidity files and search for function signatures echo "Searching Solidity files..." fd -e sol . | while read -r file; do echo "=== $file ===" grep -A 2 "function addBridgeToken" "$file" || true done echo -e "\nChecking Go bindings..." fd -e go . | while read -r file; do echo "=== $file ===" grep -A 2 "AddBridgeToken" "$file" || true doneLength of output: 112149
Summary by CodeRabbit
Breaking Changes
addBridgeToken
function across multiple contracts and interfaces_channelIBC
parameter with_memo
AddBridgeTokenEvent
to use_memo
instead of_channelIBC
Deprecation
FxOriginatedTokenEvent
inFxBridgeLogicETH
contract