[Access] Get Block endpoint is missing the system collection #5049
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o AndriiDiachuk/4584-get-block-endpoint-is-missing-system-collection
|
In this PR were covered happy paths for both methods and wrong input cases for |
AndriiDiachuk
requested review from
gomisha,
yhassanzadeh13 and
peterargue
as code owners
…lear that it's return system tx for block, generated mocks, fixed test
peterargue
reviewed
Nov 24, 2023
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5049 +/- ##
==========================================
+ Coverage 56.28% 58.93% +2.64%
==========================================
Files 976 820 -156
Lines 90985 76855 -14130
==========================================
- Hits 51212 45293 -5919
+ Misses 35966 28287 -7679
+ Partials 3807 3275 -532
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…o AndriiDiachuk/4584-get-block-endpoint-is-missing-system-collection
peterargue
approved these changes
Nov 27, 2023
…issing-system-collection
…issing-system-collection
…o AndriiDiachuk/4584-get-block-endpoint-is-missing-system-collection
…-collection' of https://github.com/AndriiDiachuk/flow-go into AndriiDiachuk/4584-get-block-endpoint-is-missing-system-collection
durkmurder
reviewed
Nov 28, 2023
durkmurder
reviewed
Nov 28, 2023
durkmurder
approved these changes
Nov 28, 2023
| @@ -355,3 +355,5 @@ require ( | |||
| replace github.com/onflow/flow-go => ../ | |||
|
|
|||
| replace github.com/onflow/flow-go/insecure => ../insecure | |||
|
|
|||
| replace github.com/onflow/flow-emulator v0.54.1-0.20231110220143-28061d9b37e7 => github.com/AndriiDiachuk/flow-emulator v0.0.0-20231128135645-f5cda716a283 | |||
There was a problem hiding this comment.
replaces pointing to a remote repo outside of the onflow org to master.
Can we remove this replace statement and instead add a require statement for the version we need, referencing the onflow/flow-emulator repo? (Maybe as part of #4957.)
In this case, it seems like the flow-go changes were backward-incompatible, but the flow-emulator changes were backward-compatible, so merging the flow-emulator change (without dependency modifications) first would save us some steps.
In cases when both are backward-incompatible, I suggest we keep dependency changes on a branch until we are at least referencing a commit/tag in the onflow org version of the repo.
There was a problem hiding this comment.
@jordanschalm Thanks for comment. I decided to do this kind of thing because of problems with github.com/onflow/flow-go/crypto version. In the flow-go on master version of crypto is v0.24.9 while in the flow-emulator on master crypto became v0.24.10. Because of this integration tests were failing. In my PR(onflow/flow-emulator#514) I downgraded version of crypto to be the same as on the flow-go master. I tried to create branch from the last commit on flow-emulator where the crypto version was v0.24.9, but there were issues with other dependencies. So if it's possible to merge my PR on flow-emulator with v0.24.9 crypto I will remove replace statement. If you have other options, will be glad to hear them.
jordanschalm
added a commit
that referenced
this pull request
Nov 28, 2023
* define direct call types * update docs * remove unnecessary event types * . * add more contract types for testing * fix tests * skip test, requires funding (implemented in follow up PR) * unskip test * check error * • extended compile-time checks for telemetry consumer enforcing that all happy-path interfaces are implemented • extended subscriptions of telemetry consumer to receive missing events * reduced notifications consumer interface in PaceMaker, as it only emits events from `hotstuff.ParticipantConsumer` * update tests * Refactor event emmision code * moves fixtures from insecure to p2ptest * adds gossipsub message fixture function * refactors test fixtures * creates GossipSubRpcFixture * adds documentation * adds godoc * Update network/p2p/unicast/README.MD Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com> * expose more test utility * lint * add register store * update LastFinalizedAndExecutedHeight * convert storage.ErrNotFound to nil * fix lint * fixes the logger * moving ready to the select-case * moving ready to select-case * revises the error by update loop to make it irrecoverable * clean up benchmarking * adds select case for startup of subscription provider * Update network/p2p/scoring/subscription_provider.go Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com> * adds documentation to subscription record cache * fixes a bug with subscription record id * adds more documentation for current cycle * untangled Register API handler * fix epoch event docs * chnages per suggestions * remove mock * [Access] Handle script canceled and timeout errors * [Access] Make script exec configurable * Make computation limit configurable on ENs * set limit setting once * undo whitespace changes * add storehouse loader * add loaders * add comments update log * add comments * update loader * add extending block snapshot * fix tests * fix lint * update interface * update tests * fix lint * update committer * update mocks * update committer tests * refactor collector * update mocks * update bootstrap * fix noop committer * fix computer_test * fix tests * refactor test * update committer tests * fix tests * Changed input parameters for new methods * Updated according to comments * Update engine/access/rpc/backend/backend_scripts_test.go Co-authored-by: Gregor G. <75445744+sideninja@users.noreply.github.com> * fix committer tests * add comment * update tests * Added implementation for execution api engine * Added tests for execution api engine * Added implementation for method for returning tx result * Fixed issues based on comments * Added accidentally removed code * Updated implementations following protobuf changes,updated mocks * Linted * [Access] Update websockets event streaming to return JSON-CDC encoded events * Added tests for sentinel errors * Update engine/execution/rpc/engine.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> * Generated mocks, added missing argument to method: * fix per suggestion * cleanup * fix mocks * lint * update NewStorageSnapshot * update mocks * update tests * refactor script engine * update mocks * fix tests * fix tests * remove scripts engine tests * update errors * update tests * refactor tests * test CreateSnapshot * add executing block snapshot * reuse convert functions * add block_end_snapshot tests * remove changes * Apply suggestions from code review Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> * add todo * Update engine/execution/storehouse/block_end_snapshot_test.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> * Update engine/execution/storehouse/block_end_snapshot_test.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> * fix lint * update comment * add back interface * update getFromStorage * handle not found case * add IsBlockExcuted * update state commmitment by block id * update mocks * fix lint * update unexecuted_loader * fix ingestion tests * fix stop control * use IsParentExecuted * chnages to error handling * Added test * fix loader tests * Fix bug in reencoding and update tests * Fixd remarks * Added test to emulate errors treated as success for cb * Added test cases for checking wrong input * Merged and Linted * Returned back apiTimeout and used it in connection factory * Updated test * Linted * remove index reporter for registersAsync * Added parameter blockID to GetSystemTransaction, so it will be more clear that it's return system tx for block, generated mocks, fixed test * Updated existing functional test to check upstream failover * use atomic pointer * Updated documentaion according to comments * Upgraded godoc * Replaced check with switch * Added event with payload to test decoding, linted * Removed replace and added current version of flow proto * Replace flow-emulator for integration tests * changes per suggestions * correct compareAndSwap * make tidy * Fixed errors * Added more comments for tests * changed version of flow-emulator * Removed comments * modifies the flow-emulator version pinned to match replace statement See: - onflow/flow-emulator#514 - #5049 --------- Co-authored-by: ramtinms <ramtin.seraj@dapperlabs.com> Co-authored-by: Bastian Müller <bastian@turbolent.com> Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co> Co-authored-by: Andrii Slisarchuk <Guitarheroua@users.noreply.github.com> Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com> Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com> Co-authored-by: Ramtin M. Seraj <ramtinms@users.noreply.github.com> Co-authored-by: Yahya Hassanzadeh, Ph.D <yhassanzadeh@ieee.org> Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr> Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com> Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com> Co-authored-by: Gregor G <75445744+sideninja@users.noreply.github.com> Co-authored-by: Jan Bernatik <jan.bernatik@dapperlabs.com> Co-authored-by: Amlandeep Bhadra <amlandeep1912@gmail.com> Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> Co-authored-by: Andrii <andriy.dyachuk95@gmail.com> Co-authored-by: UlyanaAndrukhiv <u.andrukhiv@gmail.com> Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com> Co-authored-by: Amlandeep Bhadra <koko1123@users.noreply.github.com> Co-authored-by: Andrii Slisarchuk <andriyslisarchuk@gmail.com> Co-authored-by: Kan Zhang <kan@dapperlabs.com>
peterargue
pushed a commit
that referenced
this pull request
Dec 12, 2023
…ock-endpoint-is-missing-system-collection [Access] Get Block endpoint is missing the system collection
peterargue
pushed a commit
that referenced
this pull request
Dec 12, 2023
…ock-endpoint-is-missing-system-collection [Access] Get Block endpoint is missing the system collection
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#4584
In this pull request are Introduced new methods
GetSystemTransaction,GetSystemTransactionResultfor getting system tx directly for a block.Related PR:
onflow/flow#1406
onflow/flow-emulator#514