-
Notifications
You must be signed in to change notification settings - Fork 52
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
ics-cf-solana light-client: header verification & client update; protobuf impl #510
base: hyperspace-solana
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, Header contains the entire Solana block (i.e. all shreds)? That can be rather big, no?
@@ -0,0 +1,263 @@ | |||
//! File source: solana/ledger/src/shred/legacy.rs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m guessing we can get rid of legacy support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not being use in Solana anymore? Then I think it's crucial to make sure that the encoding is still correct, which seems a bit subtle to me... But I will add a TODO for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I don’t know, but that would be my assumption. It’s probably something to verify.
There is no requirement for that, but also no limit for how many shreds can be sent. I think it's reasonable to limit the amount of shreds only to be able to restore the last entry (to calculate the hash) + (potentially) some meta information, like validators change |
…eived shreds in a header =
Check that the consensus state for the header is the same as in the storage (if have any)
/// 3. Doesn't contain duplicates | ||
/// 4. All shreds have the same slot | ||
#[derive(Clone, PartialEq, Eq, Debug)] | ||
pub struct PreCheckedShreds(Vec<Shred>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d say CheckedShreds
is a better name. PreChecked
is ambiguous and might mean that those are shreds before the check, i.e. pre being checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're not fully checked (the signature wasn't checked at least), that's why the name is like so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about PartiallyCheckedShreds
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also be SortedShreds
I suppose?
Co-authored-by: Michal Nazarewicz <[email protected]>
…te' into vmarkushin/cf-solana-client-update
…-client-update # Conflicts: # contracts/pallet-ibc/src/light_clients.rs # light-clients/icsxx-cf-solana/src/client_def.rs
Implemented initial version of the client header verification and state update, and Protobuf (de)serialisation.
The library is based on
cf-guest
crate and reuses some functionality from there. The verification of Shreds is performed by the functions taken from Solana's codebase. Some code was copy-pasted from the repo due to absence of no-std support, mentioning the original's file source path.The current version of the code doesn't contain proper tests - they will be added when the client is supported in hyperspace-solana.