Skip to content

feat: add method to get all events emitted by the contract #95

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

Merged
merged 17 commits into from
Jun 10, 2025

Conversation

isSerge
Copy link
Contributor

@isSerge isSerge commented Apr 23, 2025

Resolves #55

PR Checklist

  • Tests
  • Documentation
  • Changelog

@0xNeshi
Copy link
Collaborator

0xNeshi commented Apr 27, 2025

Hi @isSerge and thanks for the contribution. We are currently under a huge workload, so we will be reviewing the PR as soon as we can.

Copy link
Collaborator

@0xNeshi 0xNeshi left a comment

Choose a reason for hiding this comment

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

Thanks for contributing, very good job. Let's polish this a bit, and it should be ready to merge.

@isSerge isSerge requested a review from 0xNeshi June 8, 2025 01:15
Copy link
Collaborator

@0xNeshi 0xNeshi left a comment

Choose a reason for hiding this comment

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

Excellent work @isSerge 🔥

Left some nits, we can merge after fixing them

Copy link
Collaborator

@bidzyyys bidzyyys left a comment

Choose a reason for hiding this comment

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

Please apply clippy warnings and @0xNeshi comments.

@isSerge isSerge requested review from bidzyyys and 0xNeshi June 10, 2025 01:18
Copy link
Collaborator

@0xNeshi 0xNeshi left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Collaborator

@bidzyyys bidzyyys left a comment

Choose a reason for hiding this comment

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

Minor docs improvements.

let events_after_two_calls = ping.all_events();
assert_eq!(events_after_two_calls.len(), 2, "Should be 2 events");

// Second call to all_events immediately after
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Second call to all_events immediately after
// Second call to `all_events` immediately after.

"Repeated calls after two events should be consistent"
);

// Check content of the two events
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Check content of the two events
// Check content of the two events.

"Second event should be correct"
);

// Perform a read-only operation that doesn't emit events
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Perform a read-only operation that doesn't emit events
// Perform a read-only operation that does not emit events.

// Perform a read-only operation that doesn't emit events
let _pings_count = ping.sender(alice).pings_count.get();

// Third call to all_events after a read operation
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Third call to all_events after a read operation
// Third call to `all_events` after a read operation.

@bidzyyys bidzyyys merged commit e47e9bf into OpenZeppelin:main Jun 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants