Add publisher program #1854

Riateche · 2024-08-28T09:12:22Z

The new on-chain program allows publishers to push multiple price updates in a single instruction, reducing overall TPS.

Unresolved questions:

What's the good max number of prices in a publisher's buffer account?
How should we restrict access when creating a buffer account?

vercel · 2024-08-28T09:12:25Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
api-reference	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 9, 2024 9:52am
staking-v2	❌ Failed (Inspect)			Sep 9, 2024 9:52am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
xc-admin-frontend	⬜️ Ignored (Inspect)	Visit Preview		Sep 9, 2024 9:52am

ali-behjati · 2024-08-28T10:33:17Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/publisher_prices.rs

+pub struct PublisherPriceError;
+
+impl PublisherPrice {
+    pub fn new(


we should not permit feed_index of 0

Added a check.

ali-behjati · 2024-08-28T10:37:35Z

target_chains/solana/programs/pyth-price-publisher/src/instruction.rs

+        .ok_or(ProgramError::InvalidInstructionData)?;
+    ensure!(
+        ProgramError::InvalidInstructionData,
+        system_program::check_id(system.key)


let's use pubkey_eq function for this one too. check_id is not efficient.

ali-behjati · 2024-08-28T10:41:19Z

target_chains/solana/programs/pyth-price-publisher/src/instruction.rs

+    let vault = account
+        .cloned()
+        .ok_or(ProgramError::InvalidInstructionData)?;
+    let vault_pda = Pubkey::find_program_address(&[VAULT_SEED.as_bytes()], program_id);


find_program_address is inefficient as might try 256 times with different seeds. Since this is constant based on our program id, we can use declare_id to define id and have this one as a static one with a test to verify that it matches.

Replaced with create_program_address and added bumps to the instruction arguments.

ali-behjati · 2024-08-28T10:42:50Z

target_chains/solana/programs/pyth-price-publisher/src/instruction.rs

+    let buffer = account
+        .cloned()
+        .ok_or(ProgramError::InvalidInstructionData)?;
+    let buffer_pda =


same comment as above, i recommend either get bump u8 this as input instruction or put it in the account. then you can use create_program_address to only perform a single check.

Replaced with create_program_address and added bumps to the instruction arguments.

ali-behjati · 2024-08-28T10:44:14Z

target_chains/solana/programs/pyth-price-publisher/src/instruction/initialize_publisher.rs

+
+    let size = publisher_prices::size(MAX_NUM_PRICES);
+    // Deposit enough tokens to allocate the account.
+    let lamports = (Rent::get()?).minimum_balance(size) * 2;


why twice? for extra balance :?

I don't know. Removed.

ali-behjati · 2024-08-28T10:50:09Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/publisher_prices.rs

+    prices
+        .get_mut(start..end)
+        .ok_or(ExtendError::NotEnoughSpace)?
+        .copy_from_slice(new_prices);


i get why we don't cast here and it is very good. to make this copy faster, let's use sol_memcpt. Also it's good to write this assumption that we don't do any input validation for the sake of speed here.

ali-behjati · 2024-08-28T10:50:32Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/publisher_prices.rs

+    if expected_len > prices_bytes.len() {
+        return Err(ReadAccountError::InvalidNumPrices);
+    }
+    let prices = cast_slice(&prices_bytes[..expected_len]);


please add a note here too that we don't do validation here.

ali-behjati · 2024-08-28T10:51:25Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/publisher_prices.rs

+    *format == FORMAT
+}
+
+pub fn read(data: &[u8]) -> Result<(&PublisherPricesHeader, &[PublisherPrice]), ReadAccountError> {


it's a bit strange that read and read_mut have different signatures.

It's a bit awkward because I want read to return only the initialized part of the account as &[PublisherPrice] slice, but in read_mut I need to return a full slice of the remainder of the account so that we can call extend later. To make it more clear that not all values returned by read_mut are properly initialized PublisherPrice values, I decided to return &mut [u8] instead of &mut [PublisherPrice]. I thought about encapsulating it in a new struct like this:

let mut extender = read_mut(data); let publisher = extender.publisher(); extender.extend(new_prices);

But I decided to keep it simple for now. Let me know your thoughts.

ali-behjati · 2024-08-28T10:51:39Z

target_chains/solana/programs/pyth-price-publisher/src/instruction/submit_prices.rs

+#[cfg(test)]
+mod tests {
+    #[tokio::test]
+    async fn test_submit_prices() {}


maybe add some tests here too?

I've extended the tests a bit, see test_initialize_and_publish().

ali-behjati · 2024-08-28T10:52:11Z

target_chains/solana/programs/pyth-price-publisher/src/instruction/submit_prices.rs

+        header.num_prices = 0;
+    }
+    publisher_prices::extend(header, prices, data)
+        .map_err(|_| ProgramError::InvalidInstructionData)?;


i think we can introduce some errors and then convert them to program error.

Refactored error handling to use impl From from custom errors to ProgramError.

…ments

…am args

ali-behjati · 2024-09-04T18:30:38Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/publisher_config.rs

+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Zeroable, Pod)]
+#[repr(C, packed)]
+pub struct PublisherConfig {


can you add some docs on what is this account and how it's used?

Added some comments about accounts and instructions.

ali-behjati · 2024-09-05T10:04:42Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/buffer.rs

@@ -0,0 +1,156 @@
+//! The PublisherPrices account acts as the buffer for storing prices sent


please update this

ali-behjati · 2024-09-05T10:14:47Z

target_chains/solana/programs/pyth-price-publisher/src/processor.rs

+    data: &[u8],
+) -> ProgramResult {
+    match Instruction::parse(data) {
+        Ok(Instruction::Initialize) => initialize(program_id, accounts, &data[1..]),


nit: it mightly be cleaner just to parse the data then pass it.

Good idea. Refactored.

ali-behjati · 2024-09-05T10:16:20Z

target_chains/solana/programs/pyth-price-publisher/src/processor/initialize_publisher.rs

+    },
+};
+
+// TODO: restrict access?


it's now access restricted right?

Yes. Updated.

ali-behjati · 2024-09-05T10:27:53Z

target_chains/solana/programs/pyth-price-publisher/src/validate.rs

+    },
+};
+
+pub fn validate_publisher<'a>(


probably not important but as i'm not so optimist on bpf optimizer I am wondering whether we could avoid clone and use reference.

Removed clones.

ali-behjati · 2024-09-05T10:29:50Z

target_chains/solana/programs/pyth-price-publisher/src/processor/initialize_publisher.rs

+        buffer.key.to_bytes(),
+    )?;
+
+    // Write an initial Header into the buffer account to prepare it to receive prices.


Do we want to have any minimum size check for the buffer?

Right now we just check if the size is enough to fit the header. The code should work fine with any size.

…ion handlers, remove clones, fix feature

ali-behjati · 2024-09-09T08:49:43Z

target_chains/solana/programs/pyth-price-publisher/src/validate.rs

+    Ok(payer)
+}
+
+fn pubkey_eq(a: &Pubkey, b: &Pubkey) -> bool {


just for extra caution let's add inline here.

ali-behjati · 2024-09-09T08:52:15Z

target_chains/solana/programs/pyth-price-publisher/src/processor/submit_prices.rs

+    let publisher_config = publisher_config::read(*publisher_config_data)?;
+    ensure!(
+        ProgramError::InvalidArgument,
+        buffer.key.to_bytes() == publisher_config.buffer_account


let's use sol_memcmp here as well

ali-behjati · 2024-09-09T08:53:59Z

target_chains/solana/programs/pyth-price-publisher/src/processor/submit_prices.rs

+    let mut buffer_data = buffer.data.borrow_mut();
+    let (header, prices) = buffer::read_mut(*buffer_data)?;
+    let current_slot = Clock::get()?.slot;
+    if header.slot != current_slot {


let's add a comment explaining the behaviour here a bit. i think it can be somewhat cleaner if we pass slot to buffer::extend and it handles that, we can then make sure the clearing works as expected by testing it.

Moved, added a comment and a test.

ali-behjati

Nice!

jayantk · 2024-09-09T15:14:52Z

target_chains/solana/programs/pyth-price-publisher/src/processor/initialize.rs

+
+// Creates a config account that stores the authority pubkey.
+// The authority is allowed to modify publisher configs.
+pub fn initialize(


Can you document the expected accounts for all of these instructions? It's hard to tell now without reading the method implementation carefully. Also please document any assumptions around those accounts (E.g., this is uninitialized and becomes initialized after)

Addressed in #1883.

jayantk · 2024-09-09T15:15:44Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/buffer.rs

+    Ok((header, prices))
+}
+
+pub fn extend(


sort of a nitpick but I think "extend" isn't a good name for this -- "extend" usually implies that you will add more data without overwriting existing data. I think "update" or "update_prices" would be a better name.

Addressed in #1883.

jayantk · 2024-09-09T15:18:55Z

target_chains/solana/programs/pyth-price-publisher/src/processor/initialize_publisher.rs

+
+// Creates a publisher config account and stores the buffer account pubkey in it.
+// Verifies and initializes the buffer account.
+pub fn initialize_publisher(


is it permissible for a publisher to call this instruction multiple times with different buffers? If not, what's the expected process for a publisher to resize their buffer if it is not big enough?

(treat this question as a request for documentation and put the answer in the code somewhere)

Addressed in #1883.

OpenCoreCH · 2024-09-24T08:04:16Z

target_chains/solana/programs/pyth-price-publisher/src/validate.rs

+        ensure!(ProgramError::InvalidArgument, publisher_config.is_writable);
+    }
+    ensure!(
+        ProgramError::MissingRequiredSignature,


This should probably be ProgramError::InvalidSeeds or ProgramError::InvalidArgument as above?

OpenCoreCH · 2024-09-24T08:35:23Z

target_chains/solana/programs/pyth-price-publisher/src/accounts/buffer.rs

+    data: &mut [u8],
+    publisher: [u8; 32],
+) -> Result<(&mut BufferHeader, &mut [u8]), ReadAccountError> {
+    if data.len() < size_of::<BufferHeader>() {


To avoid errors, wouldn't it make more sense to ensure that data can hold at least one price? I think a buffer that can only hold the header is pretty useless, as it will never be able to publish any prices (and in the current design such a mistake would be relatively bad because the buffer cannot be resized).

Reisen and others added 4 commits August 22, 2024 16:37

feat(target_chains/solana): initial price publisher program

b91ba09

fix: exclude publisher package from the workspace

258e5ea

chore(target_chains/solana): fix warnings

b34547d

fix(target_chains/solana): update publish program

1fa5089

Riateche mentioned this pull request Aug 28, 2024

feat(target_chains/solana): initial price publisher program #1841

Closed

ali-behjati reviewed Aug 28, 2024

View reviewed changes

feat(target_chains/solana): make solana-program optional

e487dfd

vercel bot deployed to Preview – api-reference August 30, 2024 12:34 View deployment

vercel bot had a problem deploying to Preview – staking-v2 August 30, 2024 12:34 Failure

Riateche added 3 commits September 4, 2024 12:49

feat(target_chains/solana): add config account, expand tests

8f0f2ca

feat(target_chains/solana): create buffer outside the program

af2631f

refactor(target_chains/solana): add pda bumps and other minor improve…

3a7e5ea

…ments

vercel bot deployed to Preview – api-reference September 4, 2024 13:05 View deployment

vercel bot had a problem deploying to Preview – staking-v2 September 4, 2024 13:06 Failure

refactor(target_chains/solana): allow importing instruction and progr…

4df7172

…am args

Riateche force-pushed the add-publisher-program branch from 2cda8ae to 4df7172 Compare September 4, 2024 15:14

vercel bot had a problem deploying to Preview – staking-v2 September 4, 2024 15:16 Failure

vercel bot deployed to Preview – api-reference September 4, 2024 15:19 View deployment

ali-behjati reviewed Sep 4, 2024

View reviewed changes

vercel bot deployed to Preview – api-reference September 5, 2024 09:14 View deployment

vercel bot had a problem deploying to Preview – staking-v2 September 5, 2024 09:14 Failure

refactor(target_chains/solana): add format_matches to all accounts

cc0cdcb

Riateche force-pushed the add-publisher-program branch from 9db80bb to cc0cdcb Compare September 5, 2024 09:18

vercel bot deployed to Preview – api-reference September 5, 2024 09:19 View deployment

vercel bot had a problem deploying to Preview – staking-v2 September 5, 2024 09:20 Failure

feat(target_chains/solana): add publisher key to publisher config

b25700b

vercel bot deployed to Preview – api-reference September 5, 2024 09:30 View deployment

vercel bot had a problem deploying to Preview – staking-v2 September 5, 2024 09:30 Failure

ali-behjati approved these changes Sep 5, 2024

View reviewed changes

fix(target_chains/solana): set up bpf build

e88e3f6

vercel bot had a problem deploying to Preview – staking-v2 September 5, 2024 13:38 Failure

vercel bot deployed to Preview – api-reference September 5, 2024 13:40 View deployment

doc(target_chains/solana): add comments for instructions and accounts

2e90868

vercel bot deployed to Preview – api-reference September 5, 2024 16:44 View deployment

vercel bot had a problem deploying to Preview – staking-v2 September 5, 2024 16:44 Failure

refactor(target_chains/solana): parse data before passing to instruct…

9e1ce6a

…ion handlers, remove clones, fix feature

vercel bot deployed to Preview – api-reference September 6, 2024 10:15 View deployment

vercel bot had a problem deploying to Preview – staking-v2 September 6, 2024 10:16 Failure

ali-behjati reviewed Sep 9, 2024

View reviewed changes

refactor(target_chains/solana): address feedback

aaa613a

vercel bot had a problem deploying to Preview – staking-v2 September 9, 2024 09:50 Failure

vercel bot deployed to Preview – api-reference September 9, 2024 09:52 View deployment

ali-behjati approved these changes Sep 9, 2024

View reviewed changes

Riateche merged commit 2aef1ca into main Sep 9, 2024
5 of 6 checks passed

Riateche deleted the add-publisher-program branch September 9, 2024 11:29

jayantk reviewed Sep 9, 2024

View reviewed changes

OpenCoreCH reviewed Sep 25, 2024

View reviewed changes

		@@ -0,0 +1,156 @@
		//! The PublisherPrices account acts as the buffer for storing prices sent

Add publisher program #1854

Add publisher program #1854

Uh oh!

Conversation

Riateche commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Riateche commented Aug 28, 2024 •

edited

Loading

vercel bot commented Aug 28, 2024 •

edited

Loading

jayantk Sep 9, 2024 •

edited

Loading