multiple contracts deployed via deployContract cannot interact with each other

[jgeary]

Edit: see this repo with the example - https://github.com/jgeary/one-off-forge-issue

Let's say theres an old protocol (0.4.24) and a new protocol I'm building (0.8.10). The old protocol consists of ContractA and ContractB. ContractB needs to make some function calls to ContractA. The new protocol is just simply ContractC which interacts through an interface with ContractB. Now I want to write an integration test for this system.

What I want to do is use deployCode twice, once for ContractA and again for ContractB, then create a new instance of ContractC natively, then interact with them all and make some assertions.

It seems like this is impossible, and it specifically breaks down when ContractB attempts to make a call to ContractA. This causes a revert with no reason. See example below.

AB.sol:

pragma solidity 0.4.24;

contract Ownable {
    address public owner;

    constructor() public {
        owner = msg.sender;
    }

    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }

    function renounceOwnership() public onlyOwner {
        owner = address(0);
    }

    function transferOwnership(address _newOwner) public onlyOwner {
        _transferOwnership(_newOwner);
    }

    function _transferOwnership(address _newOwner) internal {
        require(_newOwner != address(0));
        owner = _newOwner;
    }
}

contract A is Ownable {}

contract B {
    A a;

    constructor(A _a) {
        a = _a;
    }

    function step1() external {
        require(
            true,
            "flip this to false to test that we're getting to this point"
        );
        require(
            a.owner() == address(this),
            "a.owner() reverts so the condition is never evaluated and this error message is not shown"
        );
        require(
            false,
            "this error message is also not shown because above line reverts"
        );
        a.transferOwnership(msg.sender);
    }

    function step2() external {
        require(a.owner() == msg.sender);
    }
}

C.sol:

pragma solidity 0.8.10;

interface IA {
    function transferOwnership(address _newOwner) external;

    function renounceOwnership() external;

    function owner() external returns (address);
}

interface IB {
    function step1() external;

    function step2() external;
}

contract C {
    address a;
    address b;

    constructor(address _a, address _b) {
        a = _a;
        b = _b;
    }

    function step1() external {
        IB(b).step1();
    }

    function step2() external {
        IB(b).step2();
        IA(a).renounceOwnership();
    }
}

Integration.t.sol:

pragma solidity 0.8.10;

import "ds-test/test.sol";
import {stdCheats} from "forge-std/stdlib.sol";
import "../C.sol";

contract IntegrationTest is DSTest, stdCheats {
    C internal c;

    function setUp() public {
        address a = deployCode("AB.sol:A");
        address b = deployCode("AB.sol:B", abi.encodePacked(a));

        IA(a).transferOwnership(b);
        c = new C(a, b);
    }

    function test() public {
        c.step1();
        c.step2();
    }
}

Output:

[FAIL. Reason: ] test() (gas: 4571)
Traces:
  [530621] IntegrationTest::setUp()
    ├─ [0] VM::getCode("AB.sol:A")
    │   └─ ← ()
    ├─ → new A@0xce71…c246
    │   └─ ← 384 bytes of code
    ├─ [0] VM::getCode("AB.sol:B")
    │   └─ ← ()
    ├─ → new B@0x185a…1aea
    │   └─ ← 761 bytes of code
    ├─ [891] A::transferOwnership(0x185a4dc360ce69bdccee33b3784b0282f7961aea)
    │   └─ ← ()
    ├─ → new C@0xefc5…b132
    │   └─ ← 408 bytes of code
    └─ ← ()
  [4571] IntegrationTest::test() // this is red
    ├─ [3842] C::step1() // red
    │   ├─ [3201] B::step1() // red
    │   │   └─ ← ()
    │   └─ ← ()
    └─ ← ()

Failed tests:
[FAIL. Reason: ] test() (gas: 4571)

[brockelmore]

The issue here was a solidity issue with the users implementation, where the user was using address b = deployCode("AB.sol:B", abi.encodePacked(a)); instead of: address b = deployCode("AB.sol:B", abi.encode(a));. This resulted in the constructor arg for the address of A being wrong because abi.encodePacked doesnt left pad an address with 0 bytes like abi.encode does. So when B SSTOREs in the constructor, it SSTOREs like so:

// OP [Stack]
AND             [0xffffffffffffffffffffffffffffffffffffffff, <LEFT ALIGNED ADDRESS OF A (should be right aligned)>]
PUSH1(0x00)     [0x00000000042...ce0000000000000]
SSTORE          [0x000..000, 0x00000000042...ce0000000000000]

which results in the wrong address being stored in a, making codesize checks fail in B.

[jgeary]

Thanks @brockelmore. I can confirm that this was the issue and your solution worked, which was replacing
address b = deployCode("AB.sol:B", abi.encodePacked(a)); with address b = deployCode("AB.sol:B", abi.encode(a)); and generally brushing up on solidity abi encoding docs.

I would recommend adding some documentation or tests that show the argument encoding in action.