Skip to content

Add missing events for admin functions and pool operations#233

Merged
Akshola00 merged 16 commits into
Web3Novalabs:mainfrom
wayzeek:fix-214-missing-events-incomplete-event-coverage
Sep 26, 2025
Merged

Add missing events for admin functions and pool operations#233
Akshola00 merged 16 commits into
Web3Novalabs:mainfrom
wayzeek:fix-214-missing-events-incomplete-event-coverage

Conversation

@wayzeek

@wayzeek wayzeek commented Sep 13, 2025

Copy link
Copy Markdown
Contributor

Summary

Closes #214

Fixed missing event emissions for critical admin functions and enhanced event coverage throughout the contract.

Changes Made

  • Added ValidatorConfirmationsUpdated event for set_required_validator_confirmations function
  • Added events for contract management: ContractPaused, ContractUnpaused, ContractUpgraded
  • Added events for validator management: ValidatorAdded, ValidatorRemoved
  • Added comprehensive pool lifecycle events with proper indexing
  • Enhanced all events with #[key] attributes for efficient frontend filtering

Testing

  • All event emission tests pass (124/124 tests)
  • Verified events are properly emitted with correct data
  • Fixed test compilation issues and event assertions

This ensures full transparency and auditability of all state-changing operations in the contract.

- Updated `starknet-foundry` version from 0.40.0 to 0.46.0 in `.tool-versions`.
- Updated `snforge_scarb_plugin` and `snforge_std` versions to 0.46.0 in `Scarb.lock` and `Scarb.toml`.
- Added new events for contract management and pool lifecycle in `predifi.cairo` and `events.cairo`.
- Refactored event emission logic to include new events such as `ContractPaused`, `ContractUnpaused`, `PoolCreated`, and `ValidatorConfirmationsUpdated`.
- Updated integration tests to validate new event emissions and refactored user address constants for better readability.
- Fix validator reference in validator removal test
- Update upgrade test to handle multiple events (OZ + custom)
- Ensure all tests pass (124/124)
@vercel

vercel Bot commented Sep 13, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
nova-one-7r2r Ready Ready Preview Comment Sep 24, 2025 2:28pm

@Akshola00 Akshola00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

revert the unnecessary changes, emit all events, delete the test event file and test event inside function and we should be good to go. thanks for your contribution

Comment thread tests/test_events.cairo Outdated
Comment thread tests/integration_test.cairo Outdated
Comment thread .tool-versions
Comment thread src/base/events.cairo Outdated
Comment thread src/predifi.cairo
Comment thread tests/integration_test.cairo
Comment thread tests/integration_test.cairo
Comment thread tests/test_contract.cairo Outdated
Comment thread tests/test_utils.cairo
- Removed obsolete events: `ContractUpgraded` and `DisputeThresholdUpdated`.
- Added new event emissions for `PoolCreationFeeCollected`, `CreatorFeesCollected`, and `ValidatorFeesDistributed`.
- Updated event documentation and logic to enhance clarity and maintainability.
- Introduced a new method to retrieve the pool creator when collecting creator fees.
- Updated event emission to use the retrieved creator instead of the pool's creator property.
- Updated `collect_pool_creation_fee` method to include `pool_id` as a parameter.
- Adjusted fee collection logic to emit the pool ID during fee collection.
- Modified tests to reflect the new method signature and ensure correct fee collection behavior.
- Streamlined event emission syntax for better readability and consistency.
- Ensured all event emissions follow a uniform structure across the contract.
- Minor adjustments to spacing for improved code clarity.
@wayzeek

wayzeek commented Sep 15, 2025

Copy link
Copy Markdown
Contributor Author

fixed @Akshola00 :)

@Akshola00 Akshola00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

great work @wayzeek this implmentation and emission of event looks great
only that you removed the test completely
pls add the tests for these events as describe in previous review so we can assert that these events were emitted and we should be good to go

@Akshola00

Copy link
Copy Markdown
Contributor

@wayzeek any updates on adding the tests back

@wayzeek

wayzeek commented Sep 20, 2025

Copy link
Copy Markdown
Contributor Author

@Akshola00 still on it, shouldn't take long though

- Added tests for new events: `ContractPaused`, `ContractUnpaused`, `DisputeRaised`, and `BetPlaced`.
- Improved existing tests to verify event emissions for various contract actions, including pool creation and validator actions.
- Streamlined event spy setup across multiple test cases for consistency.
@wayzeek

wayzeek commented Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

@Akshola00 just pushed, please let me know if you need more changes :)

@Akshola00

Copy link
Copy Markdown
Contributor

@wayzeek pls resolve changes and format

@wayzeek
wayzeek requested a review from Akshola00 September 24, 2025 14:27
@wayzeek

wayzeek commented Sep 24, 2025

Copy link
Copy Markdown
Contributor Author

fixed @Akshola00 :)

@wayzeek

wayzeek commented Sep 25, 2025

Copy link
Copy Markdown
Contributor Author

bump @Akshola00

@Akshola00

Copy link
Copy Markdown
Contributor

@wayzeek thanks for your contribution

@Akshola00
Akshola00 merged commit 118bfcb into Web3Novalabs:main Sep 26, 2025
5 of 6 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

Development

Successfully merging this pull request may close these issues.

Missing Events - Incomplete Event Coverage

2 participants