Compiler randomly changes parameters

[wbt]

Description

Compiled contract functions sometimes have additional parameters randomly added in from other functions. In the most recently discovered example I have, a third parameter was added to a two-parameter function, leading a migration to fail (Invalid number of parameters. Got 2 expected 3!). On inspecting the ABI in the JSON file for that contract (in \build\contracts\), I could see that both this function and the one before it were incorrectly expecting a third input, each copied up from three functions further down in the smart contract file. (There was no such parameter in any other preceding function or event). These extra parameters were also found in the other representations of the contract in the same JSON file below the ABI.

Workaround

Deleting the \build\contracts directory and recompiling the exact same Solidity code usually works, or at least changes which function will trigger migration failure due to having been compiled with the wrong number of variables. The deletion step is usually necessary because truffle compile will not necessarily recompile the smart contract if its source code has not changed.

The observed instance immediately preceding this bug report was the same smart contract but a different function than the one immediately preceding this bug report on a separate issue.

Environment

• Compiler version: solc: 0.5.9+commit.e560f70d.Emscripten.clang and maybe others
• Target EVM version (as per compiler settings): Default
• Framework/IDE (e.g. Truffle or Remix): Truffle
• EVM execution environment / backend / blockchain client: Doesn't matter, but Ganache
• Operating system: Win 10 Pro

Steps to Reproduce

Unfortunately, I do not have detailed steps to reliably reproduce the issue. Simply compiling the same code twice in sequence, with no change to the Solidity code at all, produces different results.

Therefore, I believe this is a bug in the compiler instead of in the language. Maybe there is some issue with how things are being stored in memory, and only getting cleared when memory free-space is needed (instead of when the data is no longer needed?). However, my limited mental model for how the compiler works is that there is an iterative loop over the functions, which does not mesh with the observation that extra parameters are being copied up from a later-defined function.

[chriseth]

At a first glance, this rather sounds like a truffle bug - at least the Solidity compiler never outputs an error message of the form "Invalid number of parameters. Got 2 expected 3!". Do you know how the compiler is invoked? Is it persistent in some javascript environment or is it freshly initialized on each call?

[wbt]

The "Got 2 expected 3!" output is from Truffle (or more probably, the EVM) and occurs during migration, when I attempt to call the function. That error just alerts me to the fact that there is an issue. Inspecting the compiled JSON file reveals that the compiled file is incorrect. Therefore, I believe pretty strongly that the issue cause is happening during compilation, even though the problematic symptoms don't become obvious until a slightly later step in my workflow. Depending on which functions are affected, the symptom might not become evident until much later after deployment.

The call on the contract is just reporting the error that would be expected if the compiled ABI file was considered authoritative, which appears to be the case from the perspective of the computer, even though the programmer might think that the Solidity source file should be authoritative.

This will also impact verifiability of compiled contracts.

[chriseth]

Would you be so kind and modify truffle so that it somehow logs what exactly is sent to the compiler as input? This would allow to check whether is a problem in the compiler or in truffle.

[wbt]

@chriseth Truffle does log the list of files that are sent to the compiler as input. However, even when the same files are sent twice in a row, even with the compilation results cleared out in between, the results are different. That is what makes this such a hard issue to reproduce reliably.

[chriseth]

@wbt are the files sent by name or by content? If by content, can you check that it is actually the same content? Which "binary" of solidity are you using? If you are using docker, it could also be a cache problem there.

[wbt]

@chriseth It looks like they are being sent by content.
Does the "0.5.9+commit.e560f70d.Emscripten.clang" above provide enough information about which "binary?" I'm not quite sure how to answer that question.

[chriseth]

Yes, it does. Can you maybe provide a complete log of how the compiler is used (which function is called with which value) so we can untangle this from truffle?

[wbt]

Here's what I can find on that:
Truffle creates a CompilerSupplier, calls load() and then calls the compile method of the object returned by the resolution of load(). The load() function gets a binary of the compiler and resolves with that; since we have the specific compiler package version the Truffle details don't seem to matter much. For config options, I'm specifying version: "^0.5.0" in my config file and otherwise using defaults, which appear to be optimizer: {enabled: false, runs: 200}.

[chriseth]

Do you think you can modify the truffle code locally to output everything that goes in and out of the compiler?

[wbt]

@chriseth Yes. I modified my local instance of Truffle (adding code here) to set up some crude checks on the compiler output (counting inputs of some functions, looking at just the ABI portion of the output) to dump the compiler-input version of a contract if the checks fail. The checks are based on (a) roughly parsing the input solcStandardInput, meaning the test would be useless if the input Solidity was modified incorrectly prior to being passed to the compiler, AND (b) some regularities in the input parameters of the certain functions in specific smart contracts under test, which should pick up the issue previously observed even if certain parts of the compiler input are corrupted.

I ran 100 recompilations in quick succession and did not observe the issue in that set, consistent with past experience of this as an intermittent bug.

I will leave the checks in there and try to keep an eye on it so that if it does dump a contract, I can compare at least the function(s) failing checks to the original. If the contract passed as input to the compiler matches, I'll assume the problem is in the Solidity compiler. If the corruption is visible in what's passed to Solidity, I'll assume the problem is in how Truffle prepares that contract for input to Solidity. In either case, I'll post here.

[leonardoalt]

@wbt any updates on the issue?

[wbt]

@leonardoalt Just what's noted here, and the possible connections to other issues noted in the two automatically generated references above. I've been avoiding recompilation because of this issue and the associated loss of productivity, intentionally doing what I can to not provide opportunities to observe continued recurrence. I'll be able to keep that up for a while, but not forever.

[wbt]

I suspect addressing Truffle issue 1650 might also yield some clues helpful for addressing this one, especially for any potential solver who, like myself, currently lacks a primary code author's deep understanding of just how the compilation code works.

Based on that issue, it seems like the compiler is somehow reading the existing compiler output and simply performing modifications, perhaps based on some imperfect assessment of what's changed in the source file, instead of recompiling and overwriting. When files are occasionally and unpredictably read-locked (e.g. for an automated backup process), this modify-instead-of-overwrite choice can produce erroneous results, such as the one observed in that issue, and with lower probability, maybe even this one. Because the problem observed here is pulling parameters from elsewhere and nearby in the file, it seems like there might be a problem with read pointers getting misaligned. The intermittent nature of the problem suggests the possibility of an externally-caused contributing factor.

When originally reporting these issues, my code was inside a OneDrive for automatic backup, where Windows had automatically uncontrollably moved it along with most other user documents. This may have increased the likelihood of observing the issues, if the random reads for backup purposes were a contributing factor. (It's still a bug if they were; that should be handled better.)

In a separate issue, OneDrive reverted to 2 months prior, which meant the loss of 2 months of work except for the fact that I'd manually made a separate copy elsewhere. Thanks to that backup, I lost only about a day of work, as well as trust in OneDrive for its ability to perform the core intended function. The code is now out of OneDrive at least until the next time Windows clamps down on user control to automatically and uncontrollably move things back in.

I suspect I have not observed the present Issue because of the above-described efforts to avoid recompilation, and because the bug is intermittent. Statistically, it is not surprising that I haven't observed it. However, if this issue is made more likely by unpredictable read locks leading to the development of read pointer mismatches and exposes vulnerabilities of the present design, that could also help explain why I haven't observed it recently.

[axic]

@wbt @gnidan what is the status on this? I admit haven't read every single message here.

[wbt]

I still have examples of compiled contracts where the parameters are randomly changed, but don't have reliable reproduction steps. It is also somewhat hard to detect without checking every parameter's function list completely after every compilation, against the expected set.

[chriseth]

Closing this due to inactivity. Please reopen if the issue can be reliably reproduced.