Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: forge flatten fails with missing field body at line 1 column 30987 #7554

Closed
2 tasks done
duncancmt opened this issue Apr 3, 2024 · 7 comments · Fixed by foundry-rs/compilers#169
Closed
2 tasks done
Labels
C-forge Command: forge Cmd-forge-flatten Command: forge flatten T-bug Type: bug

Comments

@duncancmt
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (dbc48ea 2024-04-03T00:17:03.697365187Z)

What command(s) is the bug in?

forge flatten

Operating System

Linux

Describe the bug

$ forge flatten src/ContractName.sol 
[⠊] Compiling...
[⠢] Compiling 23 files with 0.8.25
[⠰] Solc 0.8.25 finished in 4.26s
Compiler run successful!
Error: 
Failed to flatten: missing field `body` at line 1 column 28436

Seems related to #7069, but I have no further insights. I'm trying to flatten this contract because forge create --verify also fails with this contract and I'm about to pull my hair out.

@duncancmt duncancmt added the T-bug Type: bug label Apr 3, 2024
@Evalir
Copy link
Member

Evalir commented Apr 3, 2024

Hey @duncancmt thanks for the report! Would it be possible for you to provide a repro for this? Else it's really really hard to investigate.

cc @klkvr seems like another flattener edge case

@Evalir Evalir added the Cmd-forge-flatten Command: forge flatten label Apr 3, 2024
@zerosnacks zerosnacks added the T-to-reproduce Type: requires reproduction label Jul 12, 2024
@zerosnacks
Copy link
Member

Thanks @duncancmt for reporting, would be great to have a minimal reproduction or additional details on your contract

For now I'll mark it as can't repro due to limited details on how to reproduce the bug

Feel free to re-open with additional details / minimal reproduction

@zerosnacks zerosnacks closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2024
@duncancmt
Copy link
Author

Hi @zerosnacks . I couldn't give a repro because the contracts weren't released yet, but we just released, so I can show you. In the repo https://github.com/0xProject/0x-settler if you try forge flatten src/chains/Mainnet.sol, you'll observe the above error

@zerosnacks
Copy link
Member

zerosnacks commented Jul 16, 2024

Was able to narrow it down to the override tag in this code block

    function _uniV3ForkInfo(uint8 forkId)
        internal
        pure
        override
        returns (address factory, bytes32 initHash, uint32 callbackSelector)
    {
        // ...
    }

Where it overrides

function _uniV3ForkInfo(uint8 forkId) internal view virtual returns (address, bytes32, uint32);

This is fixed by removing override tag

This is something we should handle as it is valid Solidity

Additional action item is seeing if we can improve the error message returned

@zerosnacks zerosnacks removed the T-to-reproduce Type: requires reproduction label Jul 16, 2024
@zerosnacks zerosnacks changed the title forge flatten fails with cryptic error message forge flatten fails with missing field body at line 1 column 30987 Jul 16, 2024
@zerosnacks zerosnacks changed the title forge flatten fails with missing field body at line 1 column 30987 forge flatten fails with missing field body at line 1 column 30987 Jul 16, 2024
@zerosnacks zerosnacks added the C-forge Command: forge label Jul 16, 2024
@zerosnacks zerosnacks changed the title forge flatten fails with missing field body at line 1 column 30987 bug: forge flatten fails with missing field body at line 1 column 30987 Jul 16, 2024
@klkvr
Copy link
Member

klkvr commented Jul 16, 2024

This is fixed by either removing the override tag or converting the method to an empty implementation rather than an interface definition:

hmm, it's strange that removing override allows to fix this, as without override this is no longer valid solidity and causes forge build to fail

@zerosnacks
Copy link
Member

zerosnacks commented Jul 16, 2024

This is fixed by either removing the override tag or converting the method to an empty implementation rather than an interface definition:

hmm, it's strange that removing override allows to fix this, as without override this is no longer valid solidity and causes forge build to fail

Ah good point, override is indeed required in this situation and the compiler does throw an error when attempting to build

@klkvr
Copy link
Member

klkvr commented Jul 16, 2024

Ah good point, override is indeed required in this situation and the compiler does throw an error when attempting to build

ah it's succeeding because we are falling back to older flattener impl which does not require AST parsing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-flatten Command: forge flatten T-bug Type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants