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

Enable service events for all collection context #5845

Merged
merged 8 commits into from
May 11, 2024

Conversation

jordanschalm
Copy link
Member

@jordanschalm jordanschalm commented May 6, 2024

This PR enables service event collection for all transactions (not just the system chunk). See #5828 for reasoning.

I tested the change against onflow/flow-core-contracts#419 in f5e878b with this CI run. This runs the integration test network using a version of NodeVersionBeacon which emits a service event outside the system chunk, then validates the service event is incorporated and processed by the protocol state. We aren't ready to deploy that smart contract change, so I reverted the commit which pins the necessary smart contract change to re-enable the test after running the test.

  • Update Notion service events page
  • Validate with TestProtocolVersionUpgrade test (validated in f5e878b with this CI run)

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2024

Codecov Report

Attention: Patch coverage is 35.71429% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 55.64%. Comparing base (dceead4) to head (a8e1e6b).

Files Patch % Lines
fvm/environment/event_emitter.go 25.00% 7 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5845      +/-   ##
==========================================
- Coverage   55.66%   55.64%   -0.02%     
==========================================
  Files        1134     1134              
  Lines       89667    89661       -6     
==========================================
- Hits        49910    49891      -19     
- Misses      34965    34976      +11     
- Partials     4792     4794       +2     
Flag Coverage Δ
unittests 55.64% <35.71%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jordanschalm jordanschalm marked this pull request as ready for review May 7, 2024 18:24
@jordanschalm jordanschalm requested a review from kc1116 May 7, 2024 18:30
fvm/environment/event_emitter.go Outdated Show resolved Hide resolved
fvm/environment/event_emitter.go Outdated Show resolved Hide resolved
Co-authored-by: Alexander Hentschel <alex.hentschel@flowfoundation.org>
@jordanschalm jordanschalm added this pull request to the merge queue May 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 10, 2024
@jordanschalm jordanschalm added this pull request to the merge queue May 11, 2024
Merged via the queue into master with commit e4bdb7e May 11, 2024
55 checks passed
@jordanschalm jordanschalm deleted the jord/5828-allow-service-events-default branch May 11, 2024 00:46
@jordanschalm jordanschalm restored the jord/5828-allow-service-events-default branch May 13, 2024 19:27
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.

5 participants