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]: E2E Treats All Emitted Events as Anonymous (Ignores Event Names) #408

Open
2 of 4 tasks
0xNeshi opened this issue Nov 14, 2024 · 1 comment
Open
2 of 4 tasks
Labels
type: bug Something is not working as intended. type: ref A code update that doesn't meaningfully change functionality. type: test Changes to the testing suite.

Comments

@0xNeshi
Copy link
Collaborator

0xNeshi commented Nov 14, 2024

What happened?

The problem was first noticed here:
#352 (comment)

Explanation: we are trying to assert that the correct event was emitted when Ownable2Step::transfer_ownership was called, which should be OwnershipTransferStarted, but the wrong event was asserted to had been emitted (OwnerhipTransferred). This assertion should have failed the test run, but it somehow passed!
This is when we realized there might be a problem with our e2e tool.

After further investigation, the reason for this became evident - currently used version of alloy-sol-types (0.7.6) does not check event signature when decoding logs, in effect treating all events as anonymous.

Pasting below the main part of the investigation report:

I think I have found the reason for Ownable2Step e2e bug.

Long story short, I managed to confirm that e2e treats all events with the same parameter signature as interchangeable.
I confirmed this by swapping places for OwnershipTransferred and OwnershipTransferStarted in various event assertions, and even creating an additional throwaway event to assert for instead, modifying their signatures on each test run and verifying that the tests pass only if the expected event parameter signature matches the emitted event's parameter signature, despite the name difference.
After digging a bit deeper into e2e, I soon realized the problem lay with alloy-sol-types - the version we're using (0.7.6) does not check event signatures when decoding logs, it only checks indexed parameters (topics[1..]) and data, and that the problem was fixed with version 0.8.4.
Unfortunately, we're stuck with the older version stylus sdk uses...

In short, to solve the issue we will have to wait for Stylus SDK to start supporting alloy ^0.8.4 for this to be addressed.

platform

  • linux
  • windows
  • macos

Expected behavior

Event signatures should be taken into account when asserting that certain events were emitted.

Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines
@0xNeshi 0xNeshi added type: bug Something is not working as intended. needs triage Needs to be assigned the appropriate labels labels Nov 14, 2024
@0xNeshi 0xNeshi added type: test Changes to the testing suite. type: ref A code update that doesn't meaningfully change functionality. and removed needs triage Needs to be assigned the appropriate labels labels Nov 14, 2024
@bidzyyys bidzyyys mentioned this issue Nov 15, 2024
3 tasks
@0xNeshi
Copy link
Collaborator Author

0xNeshi commented Nov 19, 2024

We're tracking new releases of https://github.com/OffchainLabs/stylus-sdk-rs that we depend on to address this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something is not working as intended. type: ref A code update that doesn't meaningfully change functionality. type: test Changes to the testing suite.
Projects
Status: Todo
Development

No branches or pull requests

1 participant