feat(ethexe): Added separate type for network announces#4957
feat(ethexe): Added separate type for network announces#4957
Conversation
Refactor Announce to store only hashes of injected transactions. Add conversions and mocks for NetworkAnnounce. Update accept_announce and related functions to use NetworkAnnounce. Adjust types and imports accordingly throughout the codebase.
d24563c to
c9253d8
Compare
ecol-master
left a comment
There was a problem hiding this comment.
// just to remove requested changes
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce440222d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc73a634b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| db.injected_transaction(tx.tx_hash()) | ||
| .map(|tx| tx.into_verified()) | ||
| .ok_or_else(|| ComputeError::InjectedTransactionNotFound(tx.tx_hash())) |
There was a problem hiding this comment.
Validate loaded injected tx matches announce signature
This path now ignores the signature committed in AnnounceInjectedTransaction and resolves each entry only by tx_hash, so compute can execute a different signed envelope than the one referenced by the announce. Since injected transactions are stored by hash key and later writes overwrite earlier values, a subsequent write of the same payload hash with a different signature can change the recovered sender used during execution; that can alter state/promise results and make nodes depend on local write timing. Please verify the fetched transaction signature equals tx.signature() (or store/retrieve by the full announce tuple) before converting it to verified data.
Useful? React with 👍 / 👎.
Announce Hash-Only Refactor and
NetworkAnnounceWire PayloadSummary
This refactor splits announce representation into two layers:
Announcewith:injected_transactions: Vec<HashOf<InjectedTransaction>>NetworkAnnouncewith:injected_transactions: Vec<SignedInjectedTransaction>The goal is to stop carrying full injected transaction bodies in every local announce iteration and use hash references locally, while still transferring full signed transactions over the network when needed.
Before vs After
Before
Announcecontained fullVec<SignedInjectedTransaction>.After
Announcecontains onlyVec<HashOf<InjectedTransaction>>.NetworkAnnouncecarries fullVec<SignedInjectedTransaction>only for wire operations.Type-Level Changes
Announcefield changed inethexe/common/src/primitives.rs:Vec<SignedInjectedTransaction>Vec<HashOf<InjectedTransaction>>NetworkAnnouncelives inethexe/common/src/primitives.rs:block_hashparentgas_allowanceinjected_transactions: Vec<SignedInjectedTransaction>ethexe/common/src/network.rsfor network-facing API usage.Network aliases switched:
ValidatorAnnounce = ValidatorMessage<NetworkAnnounce>VerifiedAnnounce = VerifiedData<NetworkAnnounce>AnnouncesResponsenow storesVec<NetworkAnnounce>Conversion APIs on
NetworkAnnounce:impl From<&NetworkAnnounce> for Announceimpl From<NetworkAnnounce> for Announceimpl TryFrom<(Announce, Vec<SignedInjectedTransaction>)> for NetworkAnnounceimpl TryFrom<Announce> for NetworkAnnounce(for hash-only/empty-body test and utility cases)to_hash()(hash of converted localAnnounce)into_announce_persisting_injected_transactions(&db)to centralize:InjectedStorageAnnounceWhat Changed for
InjectedTransactionCore rule
InjectedTransactionhash is the canonical ID used in local announce chains.HashOf<InjectedTransaction>Practical impact
InjectedStorage) keyed by tx hash.End-to-End Data Flow
1) Producer path
2) Network announce acceptance path (consensus)
3) Missing-announces sync path
4) Compute path
5) db-sync server response path
Hashing and Identity Semantics
There are two relevant hashes now:
Local announce hash (
HashOf<Announce>)Signed validator message digest (for transport signing)
NetworkAnnouncepayload encodingNetworkAnnounce::to_hash()intentionally converts to local announce first (viaFrom<&NetworkAnnounce>), then hashes. This keeps network-side validation aligned with local canonical announce hashing.Storage Boundary (Important)
After the refactor:
AnnounceStoragestores local announce bodies with tx hash references.InjectedStoragestores fullSignedInjectedTransactionbodies.This boundary is now explicit and central:
InjectedStorage.Behavior and Error Handling Changes
Compute now explicitly errors if announce references a missing tx body:
ComputeError::InjectedTransactionNotFound(HashOf<InjectedTransaction>)db-sync announce response building explicitly errors if tx hash cannot be resolved:
InjectedTransactionMissing { hash }AnnounceInjectedTransactionsMismatch { hash, source }sourceisNetworkAnnounceFromAnnounceErrorDB integrity verifier no longer rejects non-empty announce injected data,
because non-empty hash lists in announces are valid by design.
Why this is better
Compatibility
This is a breaking storage-format change for announces.