Skip to content

Conversation

@fxamacker
Copy link
Member

@fxamacker fxamacker commented Jun 6, 2023

Closes #407
Unblocks onflow/flow-go#4417

Description

Replace JSON-CDC with CCF for encoding/decoding events.
Move test function SDKEventToFlow to unittest package.
Update flow-go to last commit in onflow/flow-go#4417.
Update Cadence to v0.39.2.

This is required for integration tests to pass in onflow/flow-go#4417.

Thanks @janezpodhostnik for looking into this and discussion in Slack. 👍


For contributor use:

  • Targeted PR against master branch
  • Linked to GitHub issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the GitHub PR explorer
  • Added appropriate labels

Also moved test function SDKEventToFlow to unittest package.
@fxamacker fxamacker added the Feature A new user feature or a new package API label Jun 6, 2023
@fxamacker fxamacker self-assigned this Jun 6, 2023
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Looks good! Just one question re: move of the functions and implications

@fxamacker fxamacker requested a review from turbolent June 6, 2023 20:07
@turbolent turbolent merged commit 3ba3526 into master Jun 6, 2023
@turbolent turbolent deleted the fxamacker/use-ccf-event-encoding branch June 6, 2023 20:11
bors bot added a commit to onflow/flow-go that referenced this pull request Jun 6, 2023
4417: [Exec] Use CCF in self-describing mode to encode events (replaces JSON-CDC) r=fxamacker a=fxamacker

Updates onflow/cadence#2283 

This PR uses CCF in fully self-describing mode, so events will encode to about 1/2 the size of JSON-CDC encoding.

Using CCF in partially self-describing mode can encode events to 1/14 the size of JSON-CDC but that requires other changes outside of CCF codec and other resulting work.

### TODO

- [x] Update this PR after #4390 is merged.
- [x] After the update, run CI tests and resolve any issues.

:heavy_check_mark: Requires PR #4390 (merged on June 2, 2023).
:heavy_check_mark: Requires PR onflow/cadence#2528
✔️ Requires PR onflow/cadence#2529
✔️ Requires PR onflow/flow-emulator#408

Co-authored-by: Faye Amacker <33205765+fxamacker@users.noreply.github.com>
Co-authored-by: Amlandeep Bhadra <amlandeep1912@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature A new user feature or a new package API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use CCF encoding for events to fix flow-go integration test failures

5 participants