change store::on_gossip_attestation to take a ref to signed attestations#278
Open
d4m014 wants to merge 2 commits intolambdaclass:mainfrom
Open
change store::on_gossip_attestation to take a ref to signed attestations#278d4m014 wants to merge 2 commits intolambdaclass:mainfrom
d4m014 wants to merge 2 commits intolambdaclass:mainfrom
Conversation
Contributor
Greptile SummaryThis PR changes Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| crates/blockchain/src/store.rs | Signature of on_gossip_attestation changed from owned SignedAttestation to &SignedAttestation; internal data field now cloned (cheap AttestationData clone) instead of moved. Logic is unchanged. |
| crates/blockchain/src/lib.rs | Both the BlockChainServer::on_gossip_attestation wrapper and its two call sites updated to pass &SignedAttestation; the expensive .clone() on the self-delivery path is now removed. |
Sequence Diagram
sequenceDiagram
participant P2P as P2P Gossip
participant H as NewAttestation Handler
participant BC as BlockChainServer
participant Store as store on_gossip_attestation
P2P->>H: "NewAttestation { attestation }"
H->>BC: "on_gossip_attestation(&msg.attestation)"
BC->>Store: "on_gossip_attestation(store, &signed_attestation)"
Note over Store: "clone only AttestationData (small), decode sig bytes from ref"
Store-->>BC: "Ok(()) / Err(StoreError)"
Note over BC: "Self-delivery path (is_aggregator only)"
BC->>Store: "on_gossip_attestation(store, &signed_attestation)"
Note over Store: "Before: signed_attestation.clone() — 3112-byte XMSS sig copied. Now: borrow only, no large clone"
Store-->>BC: "Ok(()) / Err(StoreError)"
Reviews (1): Last reviewed commit: "Merge branch 'main' into refactor/gossip..." | Re-trigger Greptile
This file contains hidden or 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
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.
Closes #271
This pr changes
on_gossip_attestationin bothstore.rsandlib.rsto accept&SignedAttestationinstead of taking ownership to avoid an unnecessary clone at the call site when self-delivering attestations togossip_signatures, since only the inner data field (a smallAttestationData) needs to be cloned to construct the Attestation.