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

[Logging] Indexed log topic differs between legacy and IR if explicitly downcast #12535

Closed
bshastry opened this issue Jan 14, 2022 · 1 comment · Fixed by #12584 or #12605
Closed

[Logging] Indexed log topic differs between legacy and IR if explicitly downcast #12535

bshastry opened this issue Jan 14, 2022 · 1 comment · Fixed by #12584 or #12605
Assignees
Labels
bug 🐛 codegen error Compiler generates invalid code. Critical.

Comments

@bshastry
Copy link
Contributor

bshastry commented Jan 14, 2022

contract C {
  event ev0(bytes1 indexed);
  constructor() {
    emit ev0(bytes1(bytes16(0x31313131313131313131313131313131)));
  }
}
// ====
// compileViaYul: also
// ----
// constructor() ->
// ~ emit ev0(bytes1): #"1"

Legacy truncates indexed log topic, while IR leaves it untruncated. This would lead to a divergence of blockchain state depending on which backend is used.

To reproduce

$ cp test.sol test/libsolidity/semanticTests
$ isoltest -t semanticTests/test
  Running via Yul:
  Expected result:
  // constructor() ->
  // ~ emit ev0(bytes1): #"1"

  Obtained result:
  // constructor() ->
  // ~ emit ev0(bytes1): #"1111111111111111"
  
  Attention: Updates on the test will apply the detected format displayed.
  
  Note that the test passed without Yul. 

Note that the divergence is maintained with/without optimizer so I would guess this may be a bug related to truncation (or lack thereof) in Sol->Yul.

@cameel
Copy link
Member

cameel commented Jan 24, 2022

@nishant-sachdeva

In case of discrepancies, Which version do we prefer? the output produced by the Old Code Generator or the one produced by the New Code generator ?

The one that matches the spec :) Value-type parameters are not hashed and the value passed to the event is just one byte (0x31, i.e. "1" in ASCII) so that's what should be stored.

IR behavior is weird here though. The value is not a hash but the original bytes16 value so somehow it does not generate code for the bytes1 conversion. Or maybe the lower bytes just aren't cleaned? Alternatively, could be a bug with how isoltest displays it. We added support for events only quite recently (#11050).

By the way, I think we should update the docs for indexed parameters as a part of this issue. Encoding of Indexed Event Parameters actually does not say anything about the encoding indexed parameters of value types. It only mentions non-values types.

In Contracts > Events it's also not stated outright, only implied:

You can add the attribute indexed to up to three parameters which adds them to a special data structure known as “topics” instead of the data part of the log. A topic can only hold a single word (32 bytes) so if you use a reference type for an indexed argument, the Keccak-256 hash of the value is stored as a topic instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment