removes shred wire layout specs from sigverify #25520
Conversation
behzadnouri
force-pushed
the
shred-sig-verify
branch
3 times, most recently
from
553222a
to
87e5f0d
Compare
| } | ||
|
|
||
| // Returns chunk of the payload which is signed. | ||
| pub(crate) fn get_signed_message(shred: &[u8]) -> Option<&[u8]> { |
There was a problem hiding this comment.
This may need to return Option<&[[u8]]> to account for a non-contiguous buffer if the signed data refers to everything protected by merkle vs just the signature for the merkle root.
There was a problem hiding this comment.
if the signed data refers to everything protected by merkle
not sure why should we want to sign everything,
but even then, in #25237 that is still a contiguous slice.
There was a problem hiding this comment.
ok, you can disregard. I was considering support for a shred layout where the merkle hashed data is non-contiguous.
| @@ -538,50 +576,39 @@ impl From<ShredData> for Shred { | |||
|
|
|||
| // Get slot, index, and type from a packet with partial deserialize | |||
| pub fn get_shred_slot_index_type( | |||
There was a problem hiding this comment.
Can get_shred_slot_index_type just be removed and replaced with layout::get_slot? It looks like the only usage outside of tests is in process_packet which only uses the slot value.
There was a problem hiding this comment.
yeah, I don't like it either, but even though it only uses slot, it effectively also validates index and shred_type and discards packets which have invalid index or shred_type:
https://github.com/solana-labs/solana/blob/2fb096c48/core/src/shred_fetch_stage.rs#L40
So it wouldn't be the same as just layout::get_slot.
There was a problem hiding this comment.
it seems ok to me to replace it given that the shred is going to go through another round of decode and verification. If you want to keep it to preserve the existing behavior I'm ok with that.
There was a problem hiding this comment.
leaning on keeping it for now since it does not intervene with current changes,
and it also gathers and submits ShredFetchStats metrics
behzadnouri
force-pushed
the
shred-sig-verify
branch
from
87e5f0d
to
a49d715
Compare
sigverify_shreds relies on wire layout specs of shreds: https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L39-L46 https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L298-L305 In preparation of solana-labs#25237 which adds a new shred variant with different layout and signed message, this commit removes shred layout specification from sigverify and instead encapsulate that in shred module.
behzadnouri
force-pushed
the
shred-sig-verify
branch
from
a49d715
to
8848fce
Compare
Codecov Report
@@ Coverage Diff @@
## master #25520 +/- ##
=========================================
Coverage 82.0% 82.0%
=========================================
Files 655 620 -35
Lines 171822 170457 -1365
Branches 335 0 -335
=========================================
- Hits 140972 139879 -1093
+ Misses 30734 30578 -156
+ Partials 116 0 -116 |
sigverify_shreds relies on wire layout specs of shreds: https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L39-L46 https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L298-L305 In preparation of solana-labs#25237 which adds a new shred variant with different layout and signed message, this commit removes shred layout specification from sigverify and instead encapsulate that in shred module.
sigverify_shreds relies on wire layout specs of shreds: https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L39-L46 https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L298-L305 In preparation of solana-labs#25237 which adds a new shred variant with different layout and signed message, this commit removes shred layout specification from sigverify and instead encapsulate that in shred module.
Problem
sigverify_shredsrelies on wire layout of shreds:https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L39-L46
https://github.com/solana-labs/solana/blob/0376ab41a/ledger/src/sigverify_shreds.rs#L298-L305
In preparation of #25237
which adds a new shred variant with different layout and signed message,
this commit removes shred layout specification from sigverify and
instead encapsulate that in shred module.
Summary of Changes
removed shred wire layout specs from sigverify