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

Integrate end to end hashed chain exchange with F3 #816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

masih
Copy link
Member

@masih masih commented Jan 9, 2025

Integrate the chain exchange mechanism with F3 host and runner. But without touching the core GPBFT.

The implementation here leaves two major TODOs: 1) chain broadcasting mechanism (currently coupled to GPBFT message broadcast), and 2) partial message validation prior to buffering (currently skipped entirely but with capped buffer sizes and re-validation by core GPBFT once the messages are complete).

The integration introduces the concept of Partial GMessage: a GMessage with chains replaced with the key to the chain. The work introduces a buffer and refill mechanism that listens to the chains discovered, un-buffers the messages having re-constructed their original GMessage and feeds them to the participation using the existing event loop.

Part of #792

@Kubuxu
Copy link
Contributor

Kubuxu commented Jan 9, 2025

Conceptually SGTM, will do another dive latter after I think it over.

@masih masih force-pushed the masih/partial-message branch 2 times, most recently from de7a49c to 5c24c8d Compare January 10, 2025 12:17
@masih masih marked this pull request as ready for review January 10, 2025 12:17
@masih masih requested a review from Kubuxu January 10, 2025 12:17
@masih masih force-pushed the masih/partial-message branch from 7f4fd75 to b087ced Compare January 10, 2025 12:28
@masih masih marked this pull request as draft January 10, 2025 13:51
@masih masih force-pushed the masih/partial-message branch from 56cd93b to f0d4176 Compare January 10, 2025 16:39
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 68.35443% with 125 lines in your changes missing coverage. Please review.

Project coverage is 69.34%. Comparing base (c4b5f6a) to head (09d3cb7).

Files with missing lines Patch % Lines
cbor_gen.go 28.76% 35 Missing and 17 partials ⚠️
partial_msg.go 81.81% 32 Missing and 6 partials ⚠️
host.go 54.54% 27 Missing and 8 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #816      +/-   ##
==========================================
+ Coverage   68.82%   69.34%   +0.52%     
==========================================
  Files          81       83       +2     
  Lines        8307     8662     +355     
==========================================
+ Hits         5717     6007     +290     
- Misses       2100     2134      +34     
- Partials      490      521      +31     
Files with missing lines Coverage Δ
chainexchange/pubsub.go 78.07% <100.00%> (+15.79%) ⬆️
msg_encoding.go 53.84% <100.00%> (ø)
host.go 65.09% <54.54%> (-0.63%) ⬇️
partial_msg.go 81.81% <81.81%> (ø)
cbor_gen.go 28.76% <28.76%> (ø)

... and 7 files with indirect coverage changes

Integrate the chain exchange mechanism with F3 host and runner. But
without touching the core GPBFT.

The implementation here leaves two major TODOs: 1) chain broadcasting
mechanism (currently coupled to GPBFT message broadcast), and 2)
partial message validation prior to buffering (currently skipped
entirely but with capped buffer sizes and re-validation by core GPBFT
once the messages are complete).

The integration introduces the concept of Partial GMessage: a GMessage
with chains replaced with the key to the chain. The work introduces a
buffer and refill mechanism that listens to the chains discovered,
un-buffers the messages having re-constructed their original GMessage
and feeds them to the participation using the existing event loop.

Part of #792
@masih masih force-pushed the masih/partial-message branch from 4de2e1c to 09d3cb7 Compare January 13, 2025 15:38
@masih masih marked this pull request as ready for review January 13, 2025 16:10
@BigLep BigLep added this to the M2: Mainnet Passive Testing milestone Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants