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

chore: Updates to match recent changes to the Block Stream definition #16099

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

jsync-swirlds
Copy link
Member

@jsync-swirlds jsync-swirlds commented Oct 22, 2024

  • Updated the block service definitions
    • Split one monolithic service into a few modular services
    • Modified the publish service to match agreed protocol.
    • Changed the publish call to send repeated block items,
      rather than one-at-a-time.
  • Merged a few other small changes

Note.

This change includes a small section in the Block Node API definition that is
removed (due to multiple PBJ bugs). This will be restored once the relevant
bug (#280) in the PBJ library is fixed.

@jsync-swirlds jsync-swirlds added the Block Streams All Issues related to block streams label Oct 22, 2024
@jsync-swirlds jsync-swirlds added this to the v0.56 milestone Oct 22, 2024
@jsync-swirlds jsync-swirlds self-assigned this Oct 22, 2024
@jsync-swirlds jsync-swirlds force-pushed the update-block-stream-from-protobufs branch from 5a345dc to e50a9f2 Compare October 22, 2024 22:51
@jsync-swirlds jsync-swirlds changed the title Updates to match recent changes to the Block Stream definition chore: Updates to match recent changes to the Block Stream definition Oct 22, 2024
Copy link

codacy-production bot commented Oct 23, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.01% (target: -1.00%) 41.18%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (1f09e6d) 96541 62843 65.09%
Head commit (2f3616a) 96545 (+4) 62854 (+11) 65.10% (+0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#16099) 17 7 41.18%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 41.17647% with 10 lines in your changes missing coverage. Please review.

Project coverage is 63.36%. Comparing base (1f09e6d) to head (2f3616a).
Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
...dera/node/app/blocks/impl/GrpcBlockItemWriter.java 0.00% 6 Missing ⚠️
...edera/node/app/blocks/impl/BlockStreamBuilder.java 50.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #16099      +/-   ##
=============================================
+ Coverage      63.35%   63.36%   +0.01%     
- Complexity     20103    20107       +4     
=============================================
  Files           2527     2527              
  Lines          93722    93726       +4     
  Branches        9811     9811              
=============================================
+ Hits           59381    59393      +12     
+ Misses         30741    30734       -7     
+ Partials        3600     3599       -1     
Files with missing lines Coverage Δ
...m/hedera/node/app/blocks/BlockItemsTranslator.java 95.83% <100.00%> (+0.08%) ⬆️
...edera/node/app/blocks/impl/TranslationContext.java 100.00% <ø> (ø)
...ode/app/blocks/impl/contexts/AirdropOpContext.java 100.00% <ø> (ø)
...a/node/app/blocks/impl/contexts/BaseOpContext.java 100.00% <ø> (ø)
...de/app/blocks/impl/contexts/ContractOpContext.java 100.00% <ø> (ø)
...node/app/blocks/impl/contexts/CryptoOpContext.java 100.00% <ø> (ø)
...a/node/app/blocks/impl/contexts/FileOpContext.java 100.00% <ø> (ø)
...a/node/app/blocks/impl/contexts/MintOpContext.java 100.00% <ø> (ø)
...a/node/app/blocks/impl/contexts/NodeOpContext.java 100.00% <ø> (ø)
...de/app/blocks/impl/contexts/ScheduleOpContext.java 100.00% <ø> (ø)
... and 6 more

... and 8 files with indirect coverage changes

Impacted file tree graph

@jsync-swirlds jsync-swirlds force-pushed the update-block-stream-from-protobufs branch 3 times, most recently from fb66386 to 80b39d9 Compare October 26, 2024 00:30
@jsync-swirlds jsync-swirlds marked this pull request as ready for review October 26, 2024 00:31
@jsync-swirlds jsync-swirlds requested review from a team as code owners October 26, 2024 00:31
Copy link

github-actions bot commented Oct 26, 2024

Node: HAPI Test (Restart) Results

9 files  1 errors  8 suites   4m 41s ⏱️
7 tests 3 ✅ 0 💤 4 ❌
8 runs  4 ✅ 0 💤 4 ❌

For more details on these parsing errors and failures, see this check.

Results for commit 2d0c151.

♻️ This comment has been updated with latest results.

mxtartaglia-sl
mxtartaglia-sl previously approved these changes Oct 29, 2024
@jsync-swirlds jsync-swirlds force-pushed the update-block-stream-from-protobufs branch 2 times, most recently from 25697f8 to 2d0c151 Compare October 29, 2024 00:36
timo0
timo0 previously approved these changes Nov 8, 2024
* Updated the block service definitions
   * Split one monolithic service into a few modular services
   * Modified the _publish_ service to match agreed protocol.
      * Removed the modification to temporarily work around
        multiple bugs in PBJ (bug 280, and an already-fixed
        bug in comment handling).
* Merged a few other small changes
   * Updates to protobuf files to fix indentation and malformed
     HTML tags.

Signed-off-by: Joseph Sinclair <joseph.sinclair@swirldslabs.com>
@jsync-swirlds jsync-swirlds dismissed stale reviews from timo0 and mxtartaglia-sl via 86cf740 November 8, 2024 17:05
@jsync-swirlds jsync-swirlds force-pushed the update-block-stream-from-protobufs branch from 2d0c151 to 86cf740 Compare November 8, 2024 17:05
* Updated block stream builder to remove exchange rates from transaction result
* Updated `BlockItemsTranslator` to read exchange rates from the `TranslationContext`.
* Updated all 12 `TranslationContext` implementations to carry the exchange rates
* Updated BlockStreamBuilder to store the exchange rates in the translation context only.
* Updated BlockStreamBuilderTest as needed
* Fixed other tests as needed.

Signed-off-by: Joseph Sinclair <joseph.sinclair@swirldslabs.com>
@jsync-swirlds jsync-swirlds force-pushed the update-block-stream-from-protobufs branch from 86cf740 to f5cd37c Compare November 8, 2024 17:56
Signed-off-by: Joseph Sinclair <joseph.sinclair@swirldslabs.com>
@jsync-swirlds jsync-swirlds merged commit ad90f11 into develop Nov 8, 2024
48 of 49 checks passed
@jsync-swirlds jsync-swirlds deleted the update-block-stream-from-protobufs branch November 8, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Block Streams All Issues related to block streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants