Skip to content

N-10: remove unused constant and modifiers#2126

Open
valera-grinenko-ai wants to merge 1 commit intomatter-labs:draft-v31from
valera-grinenko-ai:pr/N-10-redundant-code
Open

N-10: remove unused constant and modifiers#2126
valera-grinenko-ai wants to merge 1 commit intomatter-labs:draft-v31from
valera-grinenko-ai:pr/N-10-redundant-code

Conversation

@valera-grinenko-ai
Copy link
Copy Markdown

@valera-grinenko-ai valera-grinenko-ai commented Apr 7, 2026

Summary

Addressed

  • Remove unused INTEROP_BALANCE_CHANGE_VERSION constant from IAssetTrackerBase.sol (zero references in codebase)
  • Remove unused onlyCallFromBootloaderOrInteropHandler and onlyCallFromInteropCenterOrNTV modifiers from SystemContractBase.sol (zero references), along with their now-unused imports
  • Move _sendL1ToGatewayMigrationDataToL1 from AssetTrackerBase to L2AssetTracker (its only caller)
  • Move _sendGatewayToL1MigrationDataToL1 from AssetTrackerBase to GWAssetTracker (its only caller)
  • Clean up now-unused imports in AssetTrackerBase

Skipped

  • SAVED_TOTAL_SUPPLY / isSaved field: actively used in L2AssetTracker.sol:119 — audit claim is incorrect
  • Redundant imports (FinalizeDepositParams, IBridgehubBase, MigrationConfirmationData): claims don't hold for current code — these symbols are either directly used in contract bodies or no longer present
  • receive() in BaseTokenHolder: used on ZK OS chains during L2BaseTokenZKOS.initL2() which sends ETH via Address.sendValue — audit only considered the Era path where balance is set via storage
  • Chain ID check in _finalizeBridgeBurn: defense-in-depth; the block.chainid == _l1ChainId() guard makes the L1-only intent explicit at the call site even though _recordMigrationToSL is only implemented on L1

Test plan

  • Verify forge build passes for both l1-contracts and system-contracts
  • Confirm no references to removed symbols exist
  • Confirm L2AssetTracker and GWAssetTracker migration flows work with relocated functions

🤖 Generated with Claude Code

- Remove unused INTEROP_BALANCE_CHANGE_VERSION constant from
  IAssetTrackerBase.sol (zero references in codebase)
- Remove unused onlyCallFromBootloaderOrInteropHandler and
  onlyCallFromInteropCenterOrNTV modifiers from SystemContractBase.sol
  (zero references in codebase), along with their now-unused imports
- Move _sendL1ToGatewayMigrationDataToL1 from AssetTrackerBase to
  L2AssetTracker (its only caller)
- Move _sendGatewayToL1MigrationDataToL1 from AssetTrackerBase to
  GWAssetTracker (its only caller)
- Clean up now-unused imports in AssetTrackerBase

receive() in BaseTokenHolder is NOT removed: it is used on ZK OS
chains during L2BaseTokenZKOS.initL2() which sends ETH via
Address.sendValue. The audit claim only considered the Era path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@StanislavBreadless StanislavBreadless left a comment

Choose a reason for hiding this comment

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

The following is missing:

The [SAVED_TOTAL_SUPPLY](https://github.com/matter-labs/era-contracts/blob/4271d0a37d64c43697781fa272d53e6b9c9f401c/l1-contracts/contracts/bridge/asset-tracker/IAssetTrackerBase.sol#L10) struct is defined within the base contract's interface while it is only used within the L2AssetTracker contract. Additionally, there seems to be no actual need for this struct since the [bool isSaved](https://github.com/matter-labs/era-contracts/blob/84e5c59386b6b324d6547c9ad551e43f9a22fc93/l1-contracts/contracts/bridge/asset-tracker/IAssetTrackerBase.sol#L11) field is never used, since [isAssetRegistered](https://github.com/matter-labs/era-contracts/blob/4271d0a37d64c43697781fa272d53e6b9c9f401c/l1-contracts/contracts/bridge/asset-tracker/IAssetTrackerBase.sol#L24) is used for all the checks instead.

And this one:

In _finalizeBridgeBurn, [_recordMigrationToSL](https://github.com/matter-labs/era-contracts/blob/4271d0a37d64c43697781fa272d53e6b9c9f401c/l1-contracts/contracts/core/chain-asset-handler/ChainAssetHandlerBase.sol#L254) is conditionally called only when block.chainId equals L1, but this check is redundant since _recordMigrationToSL is only implemented in L1ChainAssetHandler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants