-
Notifications
You must be signed in to change notification settings - Fork 300
feat(lazer/sui): verify signatures against trusted signers #2982
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
Conversation
Co-Authored-By: Tejas Badadare <[email protected]>
…rosschain into tb/lazer/sui-storage
Co-Authored-By: Tejas Badadare <[email protected]>
…rosschain into tb/lazer/sui-storage
…and tests; verified with Sui CLI 1.53.2 Co-Authored-By: Tejas Badadare <[email protected]>
…rosschain into tb/lazer/sui-storage
… CLI 1.53.2 Co-Authored-By: Tejas Badadare <[email protected]>
…d gate updates with admin cap Co-Authored-By: Tejas Badadare <[email protected]>
…rosschain into tb/lazer/sui-storage
…dminCapability Co-Authored-By: Tejas Badadare <[email protected]>
…er via AdminCapability" This reverts commit 187f0e9.
…_lazer::init; share State; add OTW doc comments Co-Authored-By: Tejas Badadare <[email protected]>
…rosschain into tb/lazer/sui-verify
…rosschain into tb/lazer/sui-verify
The latest updates on your projects. Learn more about Vercel for GitHub.
|
use pyth_lazer::admin::{Self, AdminCap}; | ||
|
||
const ED25519_PUBKEY_LEN: u64 = 32; | ||
const SECP256K1_COMPRESSED_PUBKEY_LEN: u64 = 33; |
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.
Was this mislabeled?
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.
Gave a more accurate name
// When we have an unknown property, we do not know its length, and therefore | ||
// we cannot ignore it and parse the next properties. | ||
abort 0 // FIXME: return more granular error messages | ||
abort EInvalidUpdate // FIXME: return more granular error messages |
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.
Is this no longer a fixme?
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.
Still a FIXME since there are several other places in the parsing where the code can revert. Will be tackling this next.
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.
Seems reasonable as far as I can tell.
/// # Errors | ||
/// * `ESignerNotTrusted` - The recovered public key is not in the trusted signers list | ||
/// * `ESignerExpired` - The signer's certificate has expired | ||
public fun verify_le_ecdsa_message( |
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.
do we need to make it public :? maybe public in crate be enough (so we can test it, but users don't use 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.
Sure, figured it might be handy for users, but yeah i can't think of a scenario where they'd wanna validate without parsing.
Summary
Verify signature against trusted signers from state.
Next up i'll break up the parse function a bit so I can add better error handling.
How has this been tested?