Skip to content

Fix inheritance resolution for transitive duplicate contract names - #3071

Open
suxnju wants to merge 1 commit into
crytic:masterfrom
suxnju:master
Open

Fix inheritance resolution for transitive duplicate contract names#3071
suxnju wants to merge 1 commit into
crytic:masterfrom
suxnju:master

Conversation

@suxnju

@suxnju suxnju commented Aug 5, 2026

Copy link
Copy Markdown

Problem

Slither can resolve a Solidity base contract to the wrong declaration when a transitive import exposes another declaration with the same name. Solidity's compact AST already identifies every entry in linearizedBaseContracts and baseContracts with a referencedDeclaration, but Slither's remapping path discarded that identity and performed a name-based lookup through imported and current file scopes.

For example, consider four source files:

  • BaseLock.sol declares an abstract contract named Lock.
  • LockLibrary.sol declares a library named Lock.
  • IndirectBase.sol imports the library and uses it with using Lock for uint256.
  • Target.sol inherits both IndirectBase and the contract Lock.

The compiler identifies the intended base contract unambiguously. The previous lookup could nevertheless bind Target to the transitive Lock library. In a larger compilation this incorrect dependency can leave Slither's semantic analysis with no analyzable contracts and make analysis appear to hang.

This occurred in a verified Universal Router compilation from Ethereum. The artifact contains both a contract and a library named Lock; Slither selected the library for Dispatcher's inheritance and did not complete within the 10-second diagnostic timeout.

Change

When resolving a remapped inheritance entry, first look up the compiler-provided declaration ID in _contracts_by_id. Use that declaration when its name agrees with the AST remapping name.

The name check is intentional. It keeps the existing import-renaming and file scope lookup as a compatibility path for invalid IDs and aliased imports, while using compiler identity whenever that identity is internally consistent.

A four-file regression fixture reproduces the collision and asserts that Target inherits the Lock contract rather than the same-named library.

Tests

Minimal reproduction

The new fixture is under tests/unit/core/test_data/inheritance_resolution/transitive_duplicate_name/.
It is compiled as Solidity Standard JSON with solc 0.8.15.

Before the change, the isolated reproduction did not complete within 10 seconds. After the change, Slither builds the model and resolves:

Target -> Lock (contract), IndirectBase (contract)

The complete inheritance-resolution unit module passes:

3 passed in 0.08s

This includes the existing renamed-import and duplicate-name tests, so their fallback behavior remains covered.

Real verified on-chain contract

The fix was validated against existing Solidity Standard JSON artifacts for:

  • Chain: Ethereum mainnet (chainId=1)
  • Contract address: 0x66a9893cc07d91d95644aedd05d03f95e1dba8af
  • Fully qualified contract: src/pkgs/universal-router/contracts/UniversalRouter.sol:UniversalRouter
  • Compiler: Solidity 0.8.26+commit.8a97fa7a
  • Compilation size: 93 source files and 91 contracts

The patched source tree completed semantic construction in approximately 2.5 seconds and produced the following checked results:

Dispatcher.Lock -> Lock (contract, declaration id 1681)
UniversalRouter immediate parents -> IUniversalRouter, Dispatcher

Before the fix, the same artifact selected the Lock library and timed out during analysis.

Slither regression suite

No Solidity or inheritance-resolution regression was observed. Ruff reported All checks passed, and git diff --check completed without errors.

@suxnju
suxnju requested a review from smonicas as a code owner August 5, 2026 12:56
@CLAassistant

CLAassistant commented Aug 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

2 participants