argotorg / argotorg/solidity

`Undeclared identifier` error due to wrong topological sorting of sources in presence of a cycle

Open
#13,985 1 comment 0 reactions 0 assignees View on GitHub
bug :bug: low effort low impact must have
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
21

Description

## Description

When compiling contracts, Sourcify looks at the `settings.compilationTarget: { contracts/Mycontract.sol: MyContract }` of the metadata and only asks that target contract to be output by the compiler in the `outputSelection`:

```json
"outputSelection": {
"contracts/MyContract": {
"MyContract": [
"evm.bytecode.object",
"evm.deployedBytecode.object",
"metadata"
]
}
}
```

unlike [hardhat](https://github.com/NomicFoundation/hardhat/blob/aee50d6c94a5e8b33f0de30d87e14229ada1300c/packages/hardhat-core/src/internal/core/config/default-config.ts#L139) and [truffle](https://github.com/trufflesuite/truffle/blob/6bc7700cea5cd01af7e6ea38233dd10498255bb4/packages/compile-solidity/src/run.ts#L205) which output all contracts unless a target file is given

```json
"outputSelection": {
"*": {
"*": [
"abi",
"evm.bytecode",
"evm.deployedBytecode",
"evm.methodIdentifiers"
],
"": ["ast"]
}
}
```

We received a contract Sourcify wasn't able to compile because of this difference, but could be compiled with frameworks and deployed.

**To my understanding, a difference in the `outputSelection` should not affect the compilation as the inputs are not changed.** Therefore I believe this might be a bug.

Noting that we encountered a bug before because of the same reason, i.e. how we specify the `outputSelection`: https://github.com/ethereum/solidity/issues/12932

## Environment

- Compiler version: `v0.6.5+commit.f956cc89`
- Target EVM version (as per compiler settings): `istanbul`
- Framework/IDE (e.g. Truffle or Remix): Truffle and Hardhat
- EVM execution environment / backend / blockchain client: -
- Operating system: MacOs (reproduced on Linux)

## Steps to Reproduce

The standard JSON used by Sourcify:
https://gist.github.com/marcocastignoli/6011093fdab16749fe316597b6289424#file-standard-sourcify-json

Compile:
`./solc-macosx-amd64-v0.6.5+commit.f956cc89 --standard-json standard-sourcify.json`

gives the output with errors:
```json
{
"errors": [
{
"component": "general",
"formattedMessage": "contracts/shields.sol:112:9: Warning: This declaration shadows an existing declaration.\n uint256[] memory tokens = new uint256[](balanceOf(owner));\n ^---------------------^\ncontracts/shields.sol:59:5: The shadowed declaration is here:\n Shield[] private tokens;\n ^---------------------^\n",
"message": "This declaration shadows an existing declaration.",
"secondarySourceLocations": [
{
"end": 2232,
"file": "contracts/shields.sol",
"message": "The shadowed declaration is here:",
"start": 2209
}
],
"severity": "warning",
"sourceLocation": {
"end": 4177,
"file": "contracts/shields.sol",
"start": 4154
},
"type": "Warning"
},
{
"component": "general",
"formattedMessage": "contracts/CBKLandSale.sol:748:17: Warning: This declaration shadows an existing declaration.\n for(uint256 i = counter; i < counter + rcLength; i++) {\n ^-------^\ncontracts/CBKLandSale.sol:734:14: The shadowed declaration is here:\n for (uint256 i = 0; i < reservations.length; i++) {\n ^-------^\n",
"message": "This declaration shadows an existing declaration.",
"secondarySourceLocations": [
{
"end": 29653,
"file": "contracts/CBKLandSale.sol",
"message": "The shadowed declaration is here:",
"start": 29644
}
],
"severity": "warning",
"sourceLocation": {
"end": 30444,
"file": "contracts/CBKLandSale.sol",
"start": 30435
},
"type": "Warning"
},
{
"component": "general",
"formattedMessage": "contracts/cryptoblades.sol:471:9: Warning: This declaration shadows an existing declaration.\n int128 mintWeaponFee =\n ^------------------^\ncontracts/cryptoblades.sol:169:5: The shadowed declaration is here:\n int128 public mintWeaponFee;\n ^-------------------------^\n",
"message": "This declaration shadows an existing declaration.",
"secondarySourceLocations": [
{
"end": 6868,
"file": "contracts/cryptoblades.sol",
"message": "The shadowed declaration is here:",
"start": 6841
}
],
"severity": "warning",
"sourceLocation": {
"end": 17558,
"file": "contracts/cryptoblades.sol",
"start": 17538
},
"type": "Warning"
},
{
"component": "general",
"formattedMessage": "contracts/Blacksmith.sol:143:16: DeclarationError: Undeclared identifier.\n return SafeRandoms(links[LINK_SAFE_RANDOMS]).hasSingleSeedRequest(msg.sender, getSeed(seedId, shieldType));\n ^---------^\n",
"message": "Undeclared identifier.",
"severity": "error",
"sourceLocation": {
"end": 5317,
"file": "contracts/Blacksmith.sol",
"start": 5306
},
"type": "DeclarationError"
},
{
"component": "general",
"formattedMessage": "contracts/Blacksmith.sol:150:9: DeclarationError: Undeclared identifier.\n SafeRandoms(links[LINK_SAFE_RANDOMS]).requestSingleSeed(msg.sender, getSeed(uint(SHIELD_SEED), shieldType));\n ^---------^\n",
"message": "Undeclared identifier.",
"severity": "error",
"sourceLocation": {
"end": 5679,
"file": "contracts/Blacksmith.sol",
"start": 5668
},
"type": "DeclarationError"
},
{
"component": "general",
"formattedMessage": "contracts/Blacksmith.sol:159:24: DeclarationError: Undeclared identifier.\n uint256 seed = SafeRandoms(links[LINK_SAFE_RANDOMS]).popSingleSeed(msg.sender, getSeed(uint(SHIELD_SEED), shieldType), true, false);\n ^---------^\n",
"message": "Undeclared identifier.",
"severity": "error",
"sourceLocation": {
"end": 6103,
"file": "contracts/Blacksmith.sol",
"start": 6092
},
"type": "DeclarationError"
}
],
"sources": {}
}
```

If you change the `outputSelection` to the following, it compiles successfully:
```json
"outputSelection": {
"*": { <-- Changed
"BurningManager": [
"evm.bytecode.object",
"evm.deployedBytecode.object",
"metadata"
]
}
}
```

Contract repo: https://github.com/CryptoBlades/cryptoblades/tree/production
Commit hash of the deployed contract: `a7180afc8bfdc74a5450cc1c6362cafd627a0f26`
Target contract: `BurningManager.sol`

Additional background info about the issue: https://github.com/ethereum/sourcify/issues/928

Contributor guide

Open the contributing guide

Research direction

Start by running solc v0.6.5 with the standard JSON from the linked standard-sourcify.json gist and compare it with the altered outputSelection. Inspect the source-ordering behavior involved in the cycle around contracts/Blacksmith.sol and the BurningManager.sol target; done means the original selection compiles without the reported undeclared-identifier errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, solidity
Domain
blockchain, compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.