Skip to content

solidity_signature is wrong for structs with repeated convertible field types (shared seen set) #3064

Description

@ataberk-xyz

Describe the issue

Function.solidity_signature silently renders the wrong signature for a struct parameter when
two or more of the struct's fields have the same type and that type requires conversion
(user-defined value type, enum, or contract/interface). The first such field converts correctly; the
second and later ones emit the alias name instead of its underlying elementary type.

Because Contract.get_function_from_signature matches strictly, the affected functions become
unresolvable by signature — any correctly-derived ABI signature can never match. It fails
silently: no exception, no warning, just a signature that disagrees with the compiler.

Minimal reproduction

// Repro.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

type Amount is uint256;

struct Pair {
    Amount a;
    Amount b; // same user-defined value type as `a`
}

contract Repro {
    function f(Pair calldata p) external pure returns (uint256) {
        return Amount.unwrap(p.a) + Amount.unwrap(p.b);
    }
}
from slither import Slither

sl = Slither("Repro.sol")
fn = next(f for c in sl.contracts for f in c.functions_entry_points if f.name == "f")
print(fn.solidity_signature)
actual    f((uint256,Amount))
expected  f((uint256,uint256))     # solc --combined-json abi agrees with `expected`

The second Amount field is rendered as Amount; the first converts correctly to uint256.

It is not specific to user-defined value types

The same collision occurs for any sibling field type that needs conversion:

struct fields actual expected
Amount a; Amount b; (UDVT over uint256) fAlias((uint256,Amount)) fAlias((uint256,uint256))
Flag a; Flag b; (enum) fEnum((uint8,Flag)) fEnum((uint8,uint8))
IThing a; IThing b; (interface) fContract((address,IThing)) fContract((address,address))

Two uint256 fields are unaffected, which is why this hides: returning the type unconverted is
harmless precisely when no conversion was needed.

Root cause

slither/utils/type.pyconvert_type_for_solidity_signature_to_string creates one seen set
and threads it through the entire traversal, including across sibling struct fields:

def convert_type_for_solidity_signature_to_string(t: Type) -> str:
    seen: set[Type] = set()
    types = convert_type_for_solidity_signature(t, seen)
    return _convert_type_for_solidity_signature_to_string(types, seen)

def convert_type_for_solidity_signature(t: Type, seen: set[Type]) -> Type | list[Type]:
    if t in seen:
        return t          # <-- returns UNCONVERTED
    seen.add(t)
    ...
    if isinstance(underlying_type, Structure):
        types = [
            convert_type_for_solidity_signature(x.type, seen)   # <-- siblings share `seen`
            for x in underlying_type.elems_ordered
        ]

The guard is a self-recursion guard, and the comment above it says so — a struct that contains
itself must terminate. But seen accumulates every type visited anywhere in the traversal rather
than the current descent, so a repeat across siblings is misread as a recursive back-edge. The
membership test is ==/__hash__, not identity, so two distinct-but-equal field types collide too.

A note for whoever fixes it

The obvious fix — scope seen to the current descent (add on entry, drop on exit) — fixes all three
rows above but reintroduces non-termination on genuinely recursive structs. The shared mutable
seen is currently load-bearing across the phase boundary: _convert_type_for_solidity_signature_to_string
calls back into convert_type_for_solidity_signature when it unwraps an ArrayType, and relies on
entries left in seen by the first phase to stop struct Node { uint256 v; Node[] kids; } from
looping forever. I hit exactly this while testing.

Passing the ancestor set (a path) by value through both phases satisfies both properties. I
verified this locally against the three cases above plus a recursive struct, where it still yields
upstream's existing output (uint256,Node[]) and terminates.

Impact

Any consumer of solidity_signature for structs with repeated field types. Concretely, Uniswap V4's
PoolKey has two Currency fields (a UDVT over address) and is passed to all ten hook callbacks,
so every V4 hook contract's entry points are unresolvable by signature. In our use (matching
functions by ABI signature for analysis coverage) this silently excluded an entire contract from
analysis — the failure mode is "the function was never examined", with no error to notice.

Frequency

Deterministic — 100% reproducible.

Version

slither-analyzer 0.11.5

slither/utils/type.py on master is byte-identical to 0.11.5 in the relevant functions, so
master is affected as well.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions