Skip to content

feat(Gateway): Expose bulkSetApprovalForAll NFT#112

Merged
TuDo1403 merged 19 commits intomainnetfrom
feature/set-nft-approval
Jun 18, 2025
Merged

feat(Gateway): Expose bulkSetApprovalForAll NFT#112
TuDo1403 merged 19 commits intomainnetfrom
feature/set-nft-approval

Conversation

@TuDo1403
Copy link

@TuDo1403 TuDo1403 commented May 13, 2025

This pull request introduces a new bulkSetApprovalForAll function in two gateway contracts to streamline NFT approval management and adjusts the optimizer runs in the configuration file. Below is a breakdown of the most important changes:

New functionality for NFT approval management:

  • src/mainchain/MainchainGatewayV3.sol: Added the bulkSetApprovalForAll function, allowing admins or migrators to grant or revoke NFT transfer permissions for multiple operators in a single transaction. The function includes input validation to ensure the arrays have matching lengths.
  • src/ronin/gateway/RoninGatewayV3.sol: Added the same bulkSetApprovalForAll function as in MainchainGatewayV3, with identical functionality and validation.

Configuration adjustment:

  • foundry.toml: Reduced the optimizer_runs parameter from 200 to 10, likely to optimize for faster compilation during development or testing.

@github-actions
Copy link
Contributor

github-actions bot commented May 13, 2025

Aderyn Analysis Report

This report was generated by Aderyn, a static analysis tool built by Cyfrin, a blockchain security company. This report is not a substitute for manual audit or security review. It should not be relied upon for any purpose other than to assist in the identification of potential security vulnerabilities.

Table of Contents

Summary

Files Summary

Key Value
.sol Files 45
Total nSLOC 3490

Files Details

Filepath nSLOC
src/extensions/AssetMigration.sol 126
src/extensions/FunctionRestrictable.sol 45
src/extensions/GatewayV3.sol 63
src/extensions/MinimumWithdrawal.sol 25
src/extensions/RONTransferHelper.sol 18
src/extensions/TransparentUpgradeableProxyV2.sol 15
src/extensions/WethUnwrapper.sol 41
src/extensions/WithdrawalLimitation.sol 143
src/extensions/bridge-operator-governance/BridgeManager.sol 241
src/extensions/bridge-operator-governance/BridgeManagerCallbackRegister.sol 66
src/extensions/bridge-operator-governance/BridgeManagerQuorum.sol 45
src/extensions/bridge-operator-governance/BridgeTrackingHelper.sol 19
src/extensions/collections/HasContracts.sol 34
src/extensions/collections/HasProxyAdmin.sol 16
src/extensions/sequential-governance/CoreGovernance.sol 171
src/extensions/sequential-governance/GlobalCoreGovernance.sol 79
src/extensions/sequential-governance/governance-proposal/CommonGovernanceProposal.sol 73
src/extensions/sequential-governance/governance-proposal/GlobalGovernanceProposal.sol 39
src/extensions/sequential-governance/governance-proposal/GovernanceProposal.sol 46
src/extensions/sequential-governance/governance-relay/CommonGovernanceRelay.sol 71
src/extensions/sequential-governance/governance-relay/GlobalGovernanceRelay.sol 18
src/extensions/sequential-governance/governance-relay/GovernanceRelay.sol 16
src/legacy/SideChainGatewayBurner.sol 27
src/mainchain/MainchainBridgeManager.sol 67
src/mainchain/MainchainGatewayBatcher.sol 47
src/mainchain/MainchainGatewayV3.sol 307
src/mainchain/MainchainGatewayV3_initialize.sol 42
src/ronin/gateway/BridgeReward.sol 201
src/ronin/gateway/BridgeSlash.sol 169
src/ronin/gateway/BridgeTracking.sol 213
src/ronin/gateway/PauseEnforcer.sol 69
src/ronin/gateway/RoninBridgeManager.sol 142
src/ronin/gateway/RoninBridgeManagerConstructor.sol 51
src/ronin/gateway/RoninGatewayV3.sol 374
src/ronin/gateway/RoninGatewayV3_initialize.sol 26
src/ronin/migration/LegacyTokenMigrator.sol 71
src/tokens/erc20/WBTC.sol 33
src/tokens/erc20/WBTC_Sepolia.sol 10
src/types/Types.sol 17
src/types/operations/LibTUint256Slot.sol 83
src/utils/CommonErrors.sol 48
src/utils/ContractType.sol 19
src/utils/DeprecatedSlots.sol 7
src/utils/IdentityGuard.sol 43
src/utils/RoleAccess.sol 14
Total 3490

Issue Summary

Category No. of Issues
High 5
Low 0

High Issues

H-1: Unsafe Casting of integers

Downcasting int/uints in Solidity can be unsafe due to the potential for data loss and unintended behavior.When downcasting a larger integer type to a smaller one (e.g., uint256 to uint128), the value may exceed the range of the target type,leading to truncation and loss of significant digits. Use OpenZeppelin's SafeCast library to safely downcast integers.

1 Found Instances
  • Found in src/ronin/gateway/BridgeSlash.sol Line: 99

H-2: Yul block contains return

This causes the transaction execution to halt, and nothing after that call will execute including code following the assembly block.

2 Found Instances
  • Found in src/extensions/TransparentUpgradeableProxyV2.sol Line: 27
  • Found in src/ronin/gateway/BridgeTracking.sol Line: 63

H-3: ETH transferred without address checks

Consider introducing checks for msg.sender to ensure the recipient of the money is as intended.

2 Found Instances
  • Found in src/extensions/WethUnwrapper.sol Line: 27
  • Found in src/extensions/WethUnwrapper.sol Line: 32

H-4: Contract locks Ether without a withdraw function

It appears that the contract includes a payable function to accept Ether but lacks a corresponding function to withdraw it, which leads to the Ether being locked in the contract. To resolve this issue, please implement a public or external function that allows for the withdrawal of Ether from the contract.

1 Found Instances
  • Found in src/extensions/TransparentUpgradeableProxyV2.sol Line: 6

H-5: Reentrancy: State change after external call

Changing state after an external call can lead to re-entrancy attacks.Use the checks-effects-interactions pattern to avoid this issue.

16 Found Instances
  • Found in src/mainchain/MainchainGatewayV3.sol Line: 81
  • Found in src/mainchain/MainchainGatewayV3.sol Line: 82
  • Found in src/mainchain/MainchainGatewayV3_initialize.sol Line: 55
  • Found in src/ronin/gateway/BridgeSlash.sol Line: 94
  • Found in src/ronin/gateway/BridgeTracking.sol Line: 174
  • Found in src/ronin/gateway/BridgeTracking.sol Line: 198
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 110
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 111
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 112
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 113
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 114
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 115
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 186
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 200
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 287
  • Found in src/ronin/gateway/RoninGatewayV3.sol Line: 293

@TuDo1403 TuDo1403 force-pushed the feature/set-nft-approval branch from a2f08d6 to 8c09cf4 Compare May 13, 2025 05:04
@TuDo1403 TuDo1403 changed the title feat(Gateway): Expose setApprovalForAll NFT feat(Gateway): Expose setApprovalForAll NFT And BurnAll May 27, 2025
@TuDo1403 TuDo1403 changed the title feat(Gateway): Expose setApprovalForAll NFT And BurnAll feat(Gateway): Expose setApprovalForAll NFT And burnAll May 27, 2025
@TuDo1403 TuDo1403 changed the title feat(Gateway): Expose setApprovalForAll NFT And burnAll feat(Gateway): Expose setApprovalForAll NFT May 29, 2025
@TuDo1403 TuDo1403 force-pushed the feature/set-nft-approval branch from feceb57 to af8e715 Compare May 29, 2025 05:15
@TuDo1403 TuDo1403 changed the title feat(Gateway): Expose setApprovalForAll NFT feat(Gateway): Expose bulkSetApprovalForAll NFT May 29, 2025
Comment on lines +96 to +98
if (msg.sender != _getProxyAdmin()) {
_checkRole(_MIGRATOR_ROLE);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this function allow calls from the proxy admin?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to support upgradeToAndCall in proposal

@TuDo1403 TuDo1403 force-pushed the feature/set-nft-approval branch 2 times, most recently from 4037964 to 34a3c52 Compare June 17, 2025 09:41
Copy link
Collaborator

@huyhuynh3103 huyhuynh3103 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@TuDo1403 TuDo1403 force-pushed the feature/set-nft-approval branch from 2c545ca to 82c2515 Compare June 18, 2025 09:54
@TuDo1403 TuDo1403 force-pushed the feature/set-nft-approval branch from 5f81a99 to 611ac45 Compare June 18, 2025 10:10
@TuDo1403 TuDo1403 merged commit a305964 into mainnet Jun 18, 2025
4 checks passed
@TuDo1403 TuDo1403 deleted the feature/set-nft-approval branch June 18, 2025 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants