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

Allow qualified access to events from other contracts #14274

Merged
merged 1 commit into from
May 26, 2023

Conversation

nikola-matic
Copy link
Collaborator

@nikola-matic nikola-matic commented May 26, 2023

Fixes #13928
Fixes #14206

@nikola-matic nikola-matic requested a review from cameel May 26, 2023 09:54
@nikola-matic nikola-matic self-assigned this May 26, 2023
@cameel cameel requested review from r0qs and matheusaaguiar May 26, 2023 10:12
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR should be marked a solving #14206. It also covers #13928 - that one's closed but technically we still do solve it (maybe we should just reopen it briefly so that the PR can close it to make it clear that we did implement it in the end).

@nikola-matic nikola-matic force-pushed the enable-access-to-foreign-events branch from 1349620 to bb6aad2 Compare May 26, 2023 10:37
@nikola-matic nikola-matic force-pushed the enable-access-to-foreign-events branch from bb6aad2 to 5893e09 Compare May 26, 2023 10:53
@nikola-matic nikola-matic requested a review from cameel May 26, 2023 10:53
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original commit this is based on was a bit lacking in terms of testing:

  • The new cases are not covered by tests for Natspec, AST and ABI (see Export all events #10996).
  • We should probably add semantic tests for the new cases now that the syntax tests are not failing.

But I guess if we accepted it back then, it won't hurt to approve this as is.

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