-
Notifications
You must be signed in to change notification settings - Fork 286
feat(lazer/sui): state and initializer #2972
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
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
2c4ae46
feat(lazer/sui): specify version
tejasbadadare 69682bc
feat(lazer-sui): add state module with trusted signer storage and tests
devin-ai-integration[bot] 21d8937
Merge branch 'tb/lazer/sui-storage' of github.com:pyth-network/pyth-c…
tejasbadadare 7380293
fix
devin-ai-integration[bot] 0e70796
Merge branch 'tb/lazer/sui-storage' of github.com:pyth-network/pyth-c…
tejasbadadare f34b7fc
fix(sui-lazer): finalize state module with trusted signer management …
devin-ai-integration[bot] 112cd39
Merge branch 'tb/lazer/sui-storage' of github.com:pyth-network/pyth-c…
tejasbadadare d7bd862
chore(sui-lazer): remove warnings in state module; clean build on Sui…
devin-ai-integration[bot] 48e070d
feat(lazer/sui): add trusted signer state
tejasbadadare 8ee64ba
feat(lazer/sui): add package init and AdminCapability; share State an…
devin-ai-integration[bot] a7436fa
comments
tejasbadadare 88ef465
Merge branch 'tb/lazer/sui-storage' of github.com:pyth-network/pyth-c…
tejasbadadare e64eb21
resolve warnings
tejasbadadare 187f0e9
feat(sui-lazer): add admin-gated entry to update trusted signer via A…
devin-ai-integration[bot] ddad6d6
Revert "feat(sui-lazer): add admin-gated entry to update trusted sign…
devin-ai-integration[bot] 0fc417e
naming, make update_trusted_signer public
tejasbadadare da1a2a1
feat(sui-lazer): use OTW for AdminCap in admin::init; add OTW to pyth…
devin-ai-integration[bot] 389f5d7
lint
tejasbadadare f1b6fc4
doc
tejasbadadare b9a844d
remove unneeded otw type check
tejasbadadare 41c66dc
lint
tejasbadadare File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
module pyth_lazer::admin; | ||
use sui::types; | ||
|
||
public struct AdminCap has key, store { | ||
id: UID, | ||
} | ||
|
||
/// The `ADMIN` resource serves as the one-time witness. | ||
/// It has the `drop` ability, allowing it to be consumed immediately after use. | ||
/// See: https://move-book.com/programmability/one-time-witness | ||
public struct ADMIN has drop {} | ||
|
||
|
||
/// Initializes the module. Called at publish time. | ||
/// Creates and transfers ownership of the singular AdminCap capability to the deployer. | ||
/// Only the AdminCap owner can update the trusted signers. | ||
fun init(_: ADMIN, ctx: &mut TxContext) { | ||
let cap = AdminCap { id: object::new(ctx) }; | ||
transfer::public_transfer(cap, tx_context::sender(ctx)); | ||
} | ||
|
||
#[test_only] | ||
public fun mint_for_test(ctx: &mut TxContext): AdminCap { | ||
AdminCap { id: object::new(ctx) } | ||
} | ||
|
||
#[test_only] | ||
public fun destroy_for_test(cap: AdminCap) { | ||
let AdminCap { id } = cap; | ||
object::delete(id) | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ use pyth_lazer::i64::Self; | |
use pyth_lazer::update::{Self, Update}; | ||
use pyth_lazer::feed::{Self, Feed}; | ||
use pyth_lazer::channel::Self; | ||
use pyth_lazer::state; | ||
use sui::bcs; | ||
use sui::ecdsa_k1::secp256k1_ecrecover; | ||
|
||
|
@@ -14,12 +15,22 @@ const PAYLOAD_MAGIC: u32 = 2479346549; | |
|
||
|
||
// TODO: | ||
// initializer | ||
// administration -> admin cap, upgrade cap, governance? | ||
// storage module -> trusted signers, update fee?, treasury? | ||
// error handling | ||
// standalone verify signature function | ||
|
||
/// The `PYTH_LAZER` resource serves as the one-time witness. | ||
/// It has the `drop` ability, allowing it to be consumed immediately after use. | ||
/// See: https://move-book.com/programmability/one-time-witness | ||
public struct PYTH_LAZER has drop {} | ||
|
||
/// Initializes the module. Called at publish time. | ||
/// Creates and shares the singular State object. | ||
/// AdminCap is created and transferred in admin::init via a One-Time Witness. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm it's not here right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's happening in the admin module's init, but it should happen within the same publish PTB |
||
fun init(_: PYTH_LAZER, ctx: &mut TxContext) { | ||
let s = state::new(ctx); | ||
transfer::public_share_object(s); | ||
} | ||
|
||
/// Parse the Lazer update message and validate the signature. | ||
/// | ||
/// The parsing logic is based on the Lazer rust protocol definition defined here: | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,214 @@ | ||
module pyth_lazer::state; | ||
|
||
use pyth_lazer::admin::AdminCap; | ||
use pyth_lazer::admin; | ||
|
||
const ED25519_PUBKEY_LEN: u64 = 32; | ||
const EInvalidPubkeyLen: u64 = 1; | ||
const ESignerNotFound: u64 = 2; | ||
|
||
|
||
/// Lazer State consists of the current set of trusted signers. | ||
/// By verifying that a price update was signed by one of these public keys, | ||
/// you can validate the authenticity of a Lazer price update. | ||
/// | ||
/// The trusted signers are subject to rotations and expiry. | ||
public struct State has key, store { | ||
id: UID, | ||
trusted_signers: vector<TrustedSignerInfo>, | ||
} | ||
|
||
/// A trusted signer is comprised of a pubkey and an expiry time. | ||
/// A signer's signature should only be trusted up to timestamp `expires_at`. | ||
public struct TrustedSignerInfo has copy, drop, store { | ||
public_key: vector<u8>, | ||
expires_at: u64, | ||
} | ||
|
||
public(package) fun new(ctx: &mut TxContext): State { | ||
State { | ||
id: object::new(ctx), | ||
trusted_signers: vector::empty<TrustedSignerInfo>(), | ||
} | ||
} | ||
|
||
/// Get the trusted signer's public key | ||
public fun public_key(info: &TrustedSignerInfo): &vector<u8> { | ||
&info.public_key | ||
} | ||
|
||
/// Get the trusted signer's expiry timestamp | ||
public fun expires_at(info: &TrustedSignerInfo): u64 { | ||
info.expires_at | ||
} | ||
|
||
/// Get the list of trusted signers | ||
public fun get_trusted_signers(s: &State): &vector<TrustedSignerInfo> { | ||
&s.trusted_signers | ||
} | ||
|
||
/// Upsert a trusted signer's information or remove them. Can only be called by the AdminCap holder. | ||
/// - If the trusted signer pubkey already exists, the expires_at will be updated. | ||
/// - If the expired_at is set to zero, the trusted signer will be removed. | ||
/// - If the pubkey isn't found, it is added as a new trusted signer with the given expires_at. | ||
public fun update_trusted_signer(_: &AdminCap, s: &mut State, pubkey: vector<u8>, expires_at: u64) { | ||
assert!(vector::length(&pubkey) as u64 == ED25519_PUBKEY_LEN, EInvalidPubkeyLen); | ||
|
||
let mut maybe_idx = find_signer_index(&s.trusted_signers, &pubkey); | ||
if (expires_at == 0) { | ||
if (option::is_some(&maybe_idx)) { | ||
let idx = option::extract(&mut maybe_idx); | ||
// Remove by swapping with last (order not preserved), discard removed value | ||
let _ = vector::swap_remove(&mut s.trusted_signers, idx); | ||
tejasbadadare marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
option::destroy_none(maybe_idx); | ||
abort ESignerNotFound | ||
}; | ||
return | ||
}; | ||
|
||
if (option::is_some(&maybe_idx)) { | ||
let idx = option::extract(&mut maybe_idx); | ||
let info_ref = vector::borrow_mut(&mut s.trusted_signers, idx); | ||
info_ref.expires_at = expires_at | ||
} else { | ||
option::destroy_none(maybe_idx); | ||
vector::push_back( | ||
&mut s.trusted_signers, | ||
TrustedSignerInfo { public_key: pubkey, expires_at }, | ||
) | ||
} | ||
} | ||
|
||
fun find_signer_index(signers: &vector<TrustedSignerInfo>, target: &vector<u8>): Option<u64> { | ||
let len = vector::length(signers); | ||
let mut i: u64 = 0; | ||
while (i < (len as u64)) { | ||
let info_ref = vector::borrow(signers, i); | ||
if (*public_key(info_ref) == *target) { | ||
return option::some(i) | ||
}; | ||
i = i + 1 | ||
}; | ||
option::none() | ||
} | ||
tejasbadadare marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#[test_only] | ||
public fun new_for_test(ctx: &mut TxContext): State { | ||
State { | ||
id: object::new(ctx), | ||
trusted_signers: vector::empty<TrustedSignerInfo>(), | ||
} | ||
} | ||
|
||
#[test] | ||
public fun test_add_new_signer() { | ||
let mut ctx = tx_context::dummy(); | ||
let mut s = new_for_test(&mut ctx); | ||
let admin_cap = admin::mint_for_test(&mut ctx); | ||
|
||
let pk = x"0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20"; | ||
let expiry: u64 = 123; | ||
|
||
update_trusted_signer(&admin_cap, &mut s, pk, expiry); | ||
|
||
let signers_ref = get_trusted_signers(&s); | ||
assert!(vector::length(signers_ref) == 1, 100); | ||
let info = vector::borrow(signers_ref, 0); | ||
assert!(expires_at(info) == 123, 101); | ||
let got_pk = public_key(info); | ||
assert!(vector::length(got_pk) == (ED25519_PUBKEY_LEN as u64), 102); | ||
let State { id, trusted_signers } = s; | ||
let _ = trusted_signers; | ||
object::delete(id); | ||
admin::destroy_for_test(admin_cap); | ||
} | ||
|
||
#[test] | ||
public fun test_update_existing_signer_expiry() { | ||
let mut ctx = tx_context::dummy(); | ||
let mut s = new_for_test(&mut ctx); | ||
let admin_cap = admin::mint_for_test(&mut ctx); | ||
|
||
update_trusted_signer( | ||
&admin_cap, | ||
&mut s, | ||
x"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a", | ||
1000, | ||
); | ||
update_trusted_signer( | ||
&admin_cap, | ||
&mut s, | ||
x"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a", | ||
2000, | ||
); | ||
|
||
let signers_ref = get_trusted_signers(&s); | ||
assert!(vector::length(signers_ref) == 1, 110); | ||
let info = vector::borrow(signers_ref, 0); | ||
assert!(expires_at(info) == 2000, 111); | ||
let State { id, trusted_signers } = s; | ||
let _ = trusted_signers; | ||
object::delete(id); | ||
admin::destroy_for_test(admin_cap); | ||
} | ||
|
||
#[test] | ||
public fun test_remove_signer_by_zero_expiry() { | ||
let mut ctx = tx_context::dummy(); | ||
let mut s = new_for_test(&mut ctx); | ||
let admin_cap = admin::mint_for_test(&mut ctx); | ||
|
||
update_trusted_signer( | ||
&admin_cap, | ||
&mut s, | ||
x"0707070707070707070707070707070707070707070707070707070707070707", | ||
999, | ||
); | ||
update_trusted_signer( | ||
&admin_cap, | ||
&mut s, | ||
x"0707070707070707070707070707070707070707070707070707070707070707", | ||
0, | ||
); | ||
|
||
let signers_ref = get_trusted_signers(&s); | ||
assert!(vector::length(signers_ref) == 0, 120); | ||
let State { id, trusted_signers } = s; | ||
let _ = trusted_signers; | ||
object::delete(id); | ||
admin::destroy_for_test(admin_cap); | ||
} | ||
|
||
#[test, expected_failure(abort_code = EInvalidPubkeyLen)] | ||
public fun test_invalid_pubkey_length_rejected() { | ||
let mut ctx = tx_context::dummy(); | ||
let mut s = new_for_test(&mut ctx); | ||
let admin_cap = admin::mint_for_test(&mut ctx); | ||
|
||
let short_pk = x"010203"; | ||
update_trusted_signer(&admin_cap, &mut s, short_pk, 1); | ||
let State { id, trusted_signers } = s; | ||
let _ = trusted_signers; | ||
object::delete(id); | ||
admin::destroy_for_test(admin_cap); | ||
} | ||
|
||
#[test, expected_failure(abort_code = ESignerNotFound)] | ||
public fun test_remove_nonexistent_signer_fails() { | ||
let mut ctx = tx_context::dummy(); | ||
let mut s = new_for_test(&mut ctx); | ||
let admin_cap = admin::mint_for_test(&mut ctx); | ||
|
||
// Try to remove a signer that doesn't exist by setting expires_at to 0 | ||
update_trusted_signer( | ||
&admin_cap, | ||
&mut s, | ||
x"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", | ||
0, | ||
); | ||
let State { id, trusted_signers } = s; | ||
let _ = trusted_signers; | ||
object::delete(id); | ||
admin::destroy_for_test(admin_cap); | ||
} |
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.
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.
AdminCap can be created in other places by mistake no? If you want the admin cap to be only made by one, you need to enforce it via a type (like the cap retaining the witness, or having a phantom data or something).
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.
FWIW, the existing pyth contract also can't guard against new logic adding the ability to edit state. Any friend of state can call assert_latest_only and obtain a LatestOnly cap that lets them edit state. Currently governance and the "create price feeds" codepaths call it but others could be introduced in the future.
If the AdminCap retains the witness, doesn't the vuln just move to the witness? i.e. some codepath can be introduced in the future that can instantiate the witness, allowing instantiation of the AdminCap
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.
Hmmm you can't get the witness out of AdminCap without deconstructing it, right?
Regardless, I was thinking about the problem you are trying to solve (not being able to instantiate AdminCap twice) and I'm not sure we really need to solve it. We generally trust ourselves and not the others and in the future might have good reasons to have two or divide it in two capabilities.
The usage of witness also seems to be where you want external people to behave in a certain way and not us. (like someone is creating a Coin and we want to impose some certain characteristic there)