Consensys/surya

Graph draw method in wrong contract when using super inside a function.

vittominacori opened this issue ยท 6 comments

I'm concerning about the below behavior.

If a contract inherit from multiple contracts, it seems that graph insert a method within the wrong contract when that method is called using super.methodName.

Having the code below :

pragma solidity ^0.8.20;

abstract contract A {
    function fooA() public pure returns (string memory) {
        return "A";
    }
}

abstract contract B {}

contract C is A, B {
    function fooC() public pure returns (string memory) {
        return super.fooA();
    }
}

and then using

npx surya graph MyContract.sol | dot -Tpng > MyContract.png

produces the below result

graph

But contract B has no method fooA.

Why is it saying that there is an external call to B.fooA? Am I missing something?

GNSPS commented

Whoof!
This is definitely me needing to handle the inheritance pattern better. Hopefully, it's just me messing up the ordering of the inheritance statement. The worst-case scenario is a complete disregard for where the specific method name is inherited from.

I'll dig deeper. Thank you for reporting.

GNSPS commented

After analysis this is definitely the worst-case scenario my "super" keyword decoding in the inheritance tree is extremely dumb! ๐Ÿ™ˆ I'll fix it, might take a while.

GNSPS commented

It was a super easy fix after all! ๐Ÿ˜„ Please try v0.4.10! ๐Ÿ™

Thank you again!

Thanks. This solved the above issue but I have just another doubt.

Considering the below code (I have more complex contracts inheriting but this should reproduce the issue):

pragma solidity ^0.8.20;

abstract contract Foo {
    string private _fooA;
    string private _fooB;

    constructor(string memory fooA_) {
        _setFooA(fooA_);
    }
    
    function _setFooA(string memory fooA_) internal {
        _fooA = fooA_;
    }
    
    function _setFooB(string memory fooB_) internal {
        _fooB = fooB_;
    }
}

contract MyContract is Foo {
    constructor() Foo("A") {
        super._setFooB("B");
    }
}

Should it say that:

  • there is an internal call Foo::constructor -> Foo::_setFooA
  • there is an external call MyContract::constructor -> Foo::_setFooB

This is the output produced and seems that both the calls are inside the Foo constructor.

Am I wrong?

MyContract

Hi @GNSPS, have you had a chance to look at this behavior?

GNSPS commented

Thank you for that again. I've taken a look. Released a new version with a fix. v0.4.11. Feel free to try it out.