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

Incorrect logs emitted when downcasting to bytes2 or bytes1 #860

Closed
ChmielewskiKamil opened this issue Nov 28, 2022 · 10 comments
Closed

Incorrect logs emitted when downcasting to bytes2 or bytes1 #860

ChmielewskiKamil opened this issue Nov 28, 2022 · 10 comments

Comments

@ChmielewskiKamil
Copy link
Contributor

ChmielewskiKamil commented Nov 28, 2022

What is the problem?

Logs emitted by Echidna are incorrect. When downcasting from bytes32 to bytes2 or bytes1, Echidna prints an incorrect value.

I've created a simple contract to illustrate the issue (you can clone the repository from my GitHub):

contract WeirdLogs {
	// keccak256 hash of the default Echidna contract address
	bytes32 originalHash = keccak256(abi.encodePacked(address(this)));
	
	event OriginalHash(bytes32 originalHash);
	event HashCastedToBytes16(bytes16 originalHash);
	event HashCastedToBytes8(bytes8 originalHash);
	event HashCastedToBytes4(bytes4 originalHash);
	event HashCastedToBytes3(bytes3 originalHash);
	event HashCastedToBytes2(bytes2 originalHash);
	event HashCastedToBytes1(bytes1 orignalHash);

	function test_always_false() public {	
		emit OriginalHash(originalHash);
		emit HashCastedToBytes16(bytes16(originalHash));
		emit HashCastedToBytes8(bytes8(originalHash));
		emit HashCastedToBytes4(bytes4(originalHash));
		emit HashCastedToBytes3(bytes3(originalHash));
		emit HashCastedToBytes2(bytes2(originalHash));
		emit HashCastedToBytes1(bytes1(originalHash));
		
		assert(false);
	}
}

This is the output from running Echidna on the contract above:

test_always_false(): failed!💥  
  Call sequence:
    test_always_false()

Event sequence: Panic(1), OriginalHash(0x6662c13a7c344b8f8c0c3d766a8b6ca60a12a25c96208453b9bb163d6c8ecb56) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, HashCastedToBytes16(0x6662c13a7c344b8f8c0c3d766a8b6ca6) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, HashCastedToBytes8(0x6662c13a7c344b8f) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, HashCastedToBytes4(0x6662c13a) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, HashCastedToBytes3(0x6662c1) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, HashCastedToBytes2(«fb») from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, HashCastedToBytes1(«f») from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72
AssertionFailed(..):  passed! 🎉

If you look closely at the last two logs (HashCastedToBytes2 and HashCastedToBytes1), Echidna prints values: «fb» and «f», respectively.

Every other log is acceptable.

Is this expected behaviour? Am I missing something?

This is the output from running a similar test in Foundry:

Running 1 test for test/WeirdLogs.t.sol:WeirdLogsTest
[FAIL. Reason: Assertion violated] test_always_false() (gas: 10386)
Traces:
  [10386] WeirdLogsTest::test_always_false() 
    ├─ emit OriginalHash(originalHash: 0x6662c13a7c344b8f8c0c3d766a8b6ca60a12a25c96208453b9bb163d6c8ecb56)
    ├─ emit HashCastedToBytes16(originalHash: 0x6662c13a7c344b8f8c0c3d766a8b6ca6)
    ├─ emit HashCastedToBytes8(originalHash: 0x6662c13a7c344b8f)
    ├─ emit HashCastedToBytes4(originalHash: 0x6662c13a)
    ├─ emit HashCastedToBytes3(originalHash: 0x6662c1)
    ├─ emit HashCastedToBytes2(originalHash: 0x6662)
    ├─ emit HashCastedToBytes1(orignalHash: 0x66)
    └─ ← "Assertion violated"

Test result: FAILED. 0 passed; 1 failed; finished in 242.42µs

Failing tests:
Encountered 1 failing test in src/WeirdLogs.sol:WeirdLogs
[FAIL. Reason: Assertion violated] test_always_false() (gas: 10386)

As you can see, the output for bytes2 and bytes1 is printed correctly.

Steps to reproduce the issue:

  1. Clone the repo git clone https://github.com/ChmielewskiKamil/echidna-weird-behaviour.git
  2. cd echidna-weird-behaviour
  3. Install submodules with forge install
  4. Run Echidna echidna-test src/WeirdLogs.sol --contract WeirdLogs --config echidna_config.yaml
  5. Compare with Foundry results forge test -vvv

Expected behaviour

Echidna emits the following logs:

HashCastedToBytes2(0x6662) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, 
HashCastedToBytes1(0x66) from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72

Actual behaviour

HashCastedToBytes2(«fb») from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72, 
HashCastedToBytes1(«f») from: WeirdLogs@0x00a329c0648769A73afAc7F9381E08FB43dBEA72
@ggrieco-tob
Copy link
Member

Hi, thanks a lot for this detailed report. I believe this issue is caused by some hevm code, since Echidna will only print the events that receives from it. Can you try to reproduce the issue inside hevm so we can create a issue in their repository?

Thanks!

@ChmielewskiKamil
Copy link
Contributor Author

ChmielewskiKamil commented Nov 29, 2022

Thanks!

Can you try to reproduce the issue inside hevm so we can create a issue in their repository?

Yes, sure! I can do that.

UPDATE 1
Indeed I've managed to reproduce the issue in daptools.
I've copied the contracts from my example to a new daptools project and run dapp test -vvv. Here are the logs:

Running 1 tests for src/WeirdLogs.t.sol:WeirdLogsTest
[BAIL] test_always_false()

Failure: test_always_false
  src/WeirdLogs.t.sol:WeirdLogsTest
   ├╴constructor
   └╴test_always_false()
      ├╴OriginalHash(0x6662c13a7c344b8f8c0c3d766a8b6ca60a12a25c96208453b9bb163d6c8ecb56) (src/WeirdLogs.t.sol:24)
      ├╴HashCastedToBytes16(0x6662c13a7c344b8f8c0c3d766a8b6ca6) (src/WeirdLogs.t.sol:25)
      ├╴HashCastedToBytes8(0x6662c13a7c344b8f) (src/WeirdLogs.t.sol:26)
      ├╴HashCastedToBytes4(0x6662c13a) (src/WeirdLogs.t.sol:27)
      ├╴HashCastedToBytes3(0x6662c1) (src/WeirdLogs.t.sol:28)
      ├╴HashCastedToBytes2(«fb») (src/WeirdLogs.t.sol:29)
      ├╴HashCastedToBytes1(«f») (src/WeirdLogs.t.sol:30)
      └╴error Revert 0x4e487b710000000000000000000000000000000000000000000000000000000000000001 <source not found>

A repository with this example is available on my GitHub.

Shall I create an issue in their repository?

Thanks again!

@ggrieco-tob
Copy link
Member

Hi, please go ahead and create an issue in the dapptool repository. Keep in mind that hevm is migrating to the EF repository here so if the issue is fixed already there, there is little chance they will backport the patch. Echidna will use the new HEVM as soon as they release a stable version, but in the meantime, we are using dapptool code.

Thanks in advance!

@ChmielewskiKamil
Copy link
Contributor Author

Keep in mind that hevm is migrating to the EF repository here so if the issue is fixed already there, there is little chance they will backport the patch.

Good to know that!

Issue submitted.

@ggrieco-tob
Copy link
Member

Awesome work! Please let us know if something else fail.

@ChmielewskiKamil
Copy link
Contributor Author

Awesome work!

Thank you!

Please let us know if something else fail.

Sure thing 😎

@ggrieco-tob
Copy link
Member

@ChmielewskiKamil I'm wondering what is the status of this?

@ChmielewskiKamil
Copy link
Contributor Author

@ggrieco-tob tldr;
If possible, the code that handles the log emission in HEVM tries to decode bytes and present them as a string.

You can check out the whole conversation in the HEVM repo of the Ethereum Foundation.

The question now is whether this is a bug or a feature. For an unaware user, I would say that this is a bug.

I guess the HEVM team is waiting for our response on how to solve this issue.

@ChmielewskiKamil
Copy link
Contributor Author

ChmielewskiKamil commented Dec 19, 2022

Update December 19, 2022,

Once this HEVM PR (#134) by @d-xo is merged, the issue can be closed. Opportunistic decoding of fixed-size bytes is being removed.

Update December 20, 2022,

The above PR has been merged. Once Echidna moves to the new stable release of the HEVM, this issue will be solved*.

*There is also a chance that the fix from the EF repo will also be added to the old Daptools repo. This would resolve the issue sooner.

@ggrieco-tob
Copy link
Member

This is solved in master as we are already using hevm from ethereum. Thanks a lot @ChmielewskiKamil !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants