wormhole_attester: per-symbol PDAs for attestation state (#567)

* wormhole-attester: Add a last trading publish timestamp field

This change bumps price batch format to v3.1 with a new backwards
compatible field - last attested trading publish time. This is the last time we've
successfully attested a trading price. If no prior record exists, the
current publish time is used.

The new field is backed by a new PDA kind for the attester contract, called
'attestation state'. In these PDAs, we store metadata for every price, seeded by its pubkey. Currently, the metadata stores just the
last tradind timestamp for use with the new field.

* wormhole-attester: Use publish_time instead of attestation_time

* wormhole_attester: use prev_publish_time for non-trading prices

* wormhole_attester: per-symbol state PDAs, stop using prod accounts

* attester: Use Option to detect if previous state exists

Using Option<> for this makes fallback to latest value more convenient

* wormhole_attester: client AccountMeta typo

* wormhole_attester: fix mutability error

* wormhole_attester: stop using Option<> for on-chain state

* wormhole_attester: remove unused realloc logic for attestation state

wormhole_attester/client/src/lib.rs

@@ -48,7 +48,7 @@ use {
         load_product_account,
     },
     pyth_wormhole_attester::{
-        attestation_state::AttestationStateMapPDA,
+        attestation_state::AttestationStatePDA,
         config::{
             OldP2WConfigAccount,
             P2WConfigAccount,
@@ -325,8 +325,6 @@ pub fn gen_attest_tx(
         AccountMeta::new_readonly(system_program::id(), false),
         // config
         AccountMeta::new_readonly(p2w_config_addr, false),
-        // attestation_state
-        AccountMeta::new(AttestationStateMapPDA::key(None, &p2w_addr), false),
     ];
 
     // Batch contents and padding if applicable
@@ -334,8 +332,9 @@ pub fn gen_attest_tx(
         let mut not_padded: Vec<_> = symbols
             .iter()
             .flat_map(|s| {
+                let state_address = AttestationStatePDA::key(&s.price_addr, &p2w_addr);
                 vec![
-                    AccountMeta::new_readonly(s.product_addr, false),
+                    AccountMeta::new(state_address, false),
                     AccountMeta::new_readonly(s.price_addr, false),
                 ]
             })

wormhole_attester/program/src/attest.rs

@@ -1,9 +1,6 @@
 use {
     crate::{
-        attestation_state::{
-            AttestationState,
-            AttestationStateMapPDA,
-        },
+        attestation_state::AttestationStatePDA,
         config::P2WConfigAccount,
         message::{
             P2WMessage,
@@ -39,7 +36,6 @@ use {
     solitaire::{
         trace,
         AccountState,
-        CreationLamports,
         ExecutionContext,
         FromAccounts,
         Info,
@@ -57,7 +53,7 @@ use {
 /// Important: must be manually maintained until native Solitaire
 /// variable len vector support.
 ///
-/// The number must reflect how many pyth product/price pairs are
+/// The number must reflect how many pyth state/price pairs are
 /// expected in the Attest struct below. The constant itself is only
 /// used in the on-chain config in order for attesters to learn the
 /// correct value dynamically.
@@ -66,42 +62,41 @@ pub const P2W_MAX_BATCH_SIZE: u16 = 5;
 #[derive(FromAccounts)]
 pub struct Attest<'b> {
     // Payer also used for wormhole
-    pub payer:             Mut<Signer<Info<'b>>>,
-    pub system_program:    Info<'b>,
-    pub config:            P2WConfigAccount<'b, { AccountState::Initialized }>,
-    pub attestation_state: Mut<AttestationStateMapPDA<'b>>,
+    pub payer:          Mut<Signer<Info<'b>>>,
+    pub system_program: Info<'b>,
+    pub config:         P2WConfigAccount<'b, { AccountState::Initialized }>,
 
-    // Hardcoded product/price pairs, bypassing Solitaire's variable-length limitations
+    // Hardcoded state/price pairs, bypassing Solitaire's variable-length limitations
     // Any change to the number of accounts must include an appropriate change to P2W_MAX_BATCH_SIZE
-    pub pyth_product: Info<'b>,
-    pub pyth_price:   Info<'b>,
+    pub pyth_state: Mut<AttestationStatePDA<'b>>,
+    pub pyth_price: Info<'b>,
 
-    pub pyth_product2: Option<Info<'b>>,
-    pub pyth_price2:   Option<Info<'b>>,
+    pub pyth_state2: Option<Mut<AttestationStatePDA<'b>>>,
+    pub pyth_price2: Option<Info<'b>>,
 
-    pub pyth_product3: Option<Info<'b>>,
-    pub pyth_price3:   Option<Info<'b>>,
+    pub pyth_state3: Option<Mut<AttestationStatePDA<'b>>>,
+    pub pyth_price3: Option<Info<'b>>,
 
-    pub pyth_product4: Option<Info<'b>>,
-    pub pyth_price4:   Option<Info<'b>>,
+    pub pyth_state4: Option<Mut<AttestationStatePDA<'b>>>,
+    pub pyth_price4: Option<Info<'b>>,
 
-    pub pyth_product5: Option<Info<'b>>,
-    pub pyth_price5:   Option<Info<'b>>,
+    pub pyth_state5: Option<Mut<AttestationStatePDA<'b>>>,
+    pub pyth_price5: Option<Info<'b>>,
 
-    // Did you read the comment near `pyth_product`?
-    // pub pyth_product6: Option<Info<'b>>,
+    // Did you read the comment near `pyth_state`?
+    // pub pyth_state6: Option<Mut<AttestationStatePDA<'b>>>,
     // pub pyth_price6: Option<Info<'b>>,
 
-    // pub pyth_product7: Option<Info<'b>>,
+    // pub pyth_state7: Option<Mut<AttestationStatePDA<'b>>>,
     // pub pyth_price7: Option<Info<'b>>,
 
-    // pub pyth_product8: Option<Info<'b>>,
+    // pub pyth_state8: Option<Mut<AttestationStatePDA<'b>>>,
     // pub pyth_price8: Option<Info<'b>>,
 
-    // pub pyth_product9: Option<Info<'b>>,
+    // pub pyth_state9: Option<Mut<AttestationStatePDA<'b>>>,
     // pub pyth_price9: Option<Info<'b>>,
 
-    // pub pyth_product10: Option<Info<'b>>,
+    // pub pyth_state10: Option<Mut<AttestationStatePDA<'b>>>,
     // pub pyth_price10: Option<Info<'b>>,
     pub clock: Sysvar<'b, Clock>,
 
@@ -161,57 +156,55 @@ pub fn attest(ctx: &ExecutionContext, accs: &mut Attest, data: AttestData) -> So
 
 
     // Make the specified prices iterable
-    let price_pair_opts = [
-        Some(&accs.pyth_product),
-        Some(&accs.pyth_price),
-        accs.pyth_product2.as_ref(),
-        accs.pyth_price2.as_ref(),
-        accs.pyth_product3.as_ref(),
-        accs.pyth_price3.as_ref(),
-        accs.pyth_product4.as_ref(),
-        accs.pyth_price4.as_ref(),
-        accs.pyth_product5.as_ref(),
-        accs.pyth_price5.as_ref(),
-        // Did you read the comment near `pyth_product`?
-        // accs.pyth_product6.as_ref(),
-        // accs.pyth_price6.as_ref(),
-        // accs.pyth_product7.as_ref(),
-        // accs.pyth_price7.as_ref(),
-        // accs.pyth_product8.as_ref(),
-        // accs.pyth_price8.as_ref(),
-        // accs.pyth_product9.as_ref(),
-        // accs.pyth_price9.as_ref(),
-        // accs.pyth_product10.as_ref(),
-        // accs.pyth_price10.as_ref(),
+    let mut price_pair_opts = [
+        (Some(&mut accs.pyth_state), Some(&accs.pyth_price)),
+        (accs.pyth_state2.as_mut(), accs.pyth_price2.as_ref()),
+        (accs.pyth_state3.as_mut(), accs.pyth_price3.as_ref()),
+        (accs.pyth_state4.as_mut(), accs.pyth_price4.as_ref()),
+        (accs.pyth_state5.as_mut(), accs.pyth_price5.as_ref()),
+        // Did you read the comment near `pyth_state`?
+        // (accs.pyth_state6.as_mut(), accs.pyth_price6.as_ref()),
+        // (accs.pyth_state7.as_mut(), accs.pyth_price7.as_ref()),
+        // (accs.pyth_state8.as_mut(), accs.pyth_price8.as_ref()),
+        // (accs.pyth_state9.as_mut(), accs.pyth_price9.as_ref()),
+        // (accs.pyth_state10.as_mut(), accs.pyth_price10.as_ref()),
     ];
 
-    let price_pairs: Vec<_> = price_pair_opts.iter().filter_map(|acc| *acc).collect();
+    let price_pairs: Vec<(_, _)> = price_pair_opts
+        .iter_mut()
+        .filter_map(|pair| match pair {
+            // Only use this pair if both accounts are Some
+            (Some(state), Some(price)) => Some((state, price)),
+            _other => None,
+        })
+        .collect();
 
-    if price_pairs.len() % 2 != 0 {
-        trace!(&format!(
-            "Uneven product/price count detected: {}",
-            price_pairs.len()
-        ));
-        return Err(ProgramError::InvalidAccountData.into());
-    }
 
-    trace!("{} Pyth symbols received", price_pairs.len() / 2);
+    trace!("{} Pyth symbols received", price_pairs.len());
 
-    // Collect the validated symbols for batch serialization
-    let mut attestations = Vec::with_capacity(price_pairs.len() / 2);
+    // Collect the validated symbols here for batch serialization
+    let mut attestations = Vec::with_capacity(price_pairs.len());
 
-    for pair in price_pairs.as_slice().chunks_exact(2) {
-        let product = pair[0];
-        let price = pair[1];
+    for (state, price) in price_pairs.into_iter() {
+        // Pyth must own the price
+        if accs.config.pyth_owner != *price.owner {
+            trace!(&format!(
+                "Price {:?}: owner pubkey mismatch (expected pyth_owner {:?}, got unknown price owner {:?})",
+                price, accs.config.pyth_owner, price.owner
+            ));
+            return Err(SolitaireError::InvalidOwner(*price.owner));
+        }
 
-        if accs.config.pyth_owner != *price.owner || accs.config.pyth_owner != *product.owner {
+        // State pubkey must reproduce from the price id
+        let state_addr_from_price = AttestationStatePDA::key(price.key, ctx.program_id);
+        if state_addr_from_price != *state.0 .0.info().key {
             trace!(&format!(
-            "Pair {:?} - {:?}: pyth_owner pubkey mismatch (expected {:?}, got product owner {:?} and price owner {:?}",
-		product, price,
-            accs.config.pyth_owner, product.owner, price.owner
-        ));
-            return Err(SolitaireError::InvalidOwner(*accs.pyth_price.owner));
+                "Price {:?}: pubkey does not produce the passed state account (expected {:?} from seeds, {:?} was passed)",
+		price.key, state_addr_from_price, state.0.0.info().key
+            ));
+            return Err(ProgramError::InvalidAccountData.into());
         }
+
         let attestation_time = accs.clock.unix_timestamp;
 
         let price_data_ref = price.try_borrow_data()?;
@@ -224,53 +217,63 @@ pub fn attest(ctx: &ExecutionContext, accs: &mut Attest, data: AttestData) -> So
                 ProgramError::InvalidAccountData
             })?;
 
-        // prev_publish_time is picked if the price is not trading
-        let last_trading_publish_time = match price_struct.agg.status {
+        // Retrieve and rotate last_attested_tradind_publish_time
+
+        // Pick the value to store for the next attestation of this
+        // symbol. We use the prev_ value if the symbol is not
+        // currently being traded. The oracle marks the last known
+        // trading timestamp with it.
+        let new_last_attested_trading_publish_time = match price_struct.agg.status {
             PriceStatus::Trading => price_struct.timestamp,
             _ => price_struct.prev_timestamp,
         };
 
-        // Take a mut reference to this price's metadata
-        let state_entry: &mut AttestationState = accs
-            .attestation_state
-            .entries
-            .entry(*price.key)
-            .or_insert(AttestationState {
-                // Use the same value if no state
-                // exists for the symbol, the new value _becomes_ the
-                // last attested trading publish time
-                last_attested_trading_publish_time: last_trading_publish_time,
-            });
+        // Retrieve the timestamp saved during the previous
+        // attestation. Use the new_* value if no existind state is
+        // present on-chain
+        let current_last_attested_trading_publish_time = if state.0 .0.is_initialized() {
+            // Use the existing on-chain value
+            state.0 .0 .1.last_attested_trading_publish_time
+        } else {
+            // Fall back to the new value if the state is not initialized
+            new_last_attested_trading_publish_time
+        };
 
+        // Build an attestatioin struct for this symbol using the just decided current value
         let attestation = PriceAttestation::from_pyth_price_struct(
             Identifier::new(price.key.to_bytes()),
             attestation_time,
-            state_entry.last_attested_trading_publish_time, // Used as last_attested_publish_time
+            current_last_attested_trading_publish_time,
             price_struct,
         );
 
-
-        // update last_attested_publish_time with this price's
-        // publish_time. Yes, it may be redundant for the entry() used
-        // above in the rare first attestation edge case.
-        state_entry.last_attested_trading_publish_time = last_trading_publish_time;
-
-        // The following check is crucial against poorly ordered
-        // account inputs, e.g. [Some(prod1), Some(price1),
-        // Some(prod2), None, None, Some(price)], interpreted by
-        // earlier logic as [(prod1, price1), (prod2, price3)].
-        //
-        // Failing to verify the product/price relationship could lead
-        // to mismatched product/price metadata, which would result in
-        // a false attestation.
-        if attestation.product_id.to_bytes() != product.key.to_bytes() {
-            trace!(&format!(
-                "Price's product_id does not match the pased account (points at {:?} instead)",
-                attestation.product_id
-            ));
-            return Err(ProgramError::InvalidAccountData.into());
+        // Save the new value for the next attestation of this symbol
+        state.0 .0.last_attested_trading_publish_time = new_last_attested_trading_publish_time;
+
+        // handling of last_attested_trading_publish_time ends here
+
+        if !state.0 .0.is_initialized() {
+            // Serialize the state to learn account size for creation
+            let state_serialized = state.0 .0 .1.try_to_vec()?;
+
+            let seeds = state.self_bumped_seeds(price.key, ctx.program_id);
+            solitaire::create_account(
+                ctx,
+                state.0 .0.info(),
+                accs.payer.key,
+                solitaire::CreationLamports::Exempt,
+                state_serialized.len(),
+                ctx.program_id,
+                solitaire::IsSigned::SignedWithSeeds(&[seeds
+                    .iter()
+                    .map(|s| s.as_slice())
+                    .collect::<Vec<_>>()
+                    .as_slice()]),
+            )?;
+            trace!("Attestation state init OK");
         }
 
+
         attestations.push(attestation);
     }
 
@@ -280,51 +283,6 @@ pub fn attest(ctx: &ExecutionContext, accs: &mut Attest, data: AttestData) -> So
 
     trace!("Attestations successfully created");
 
-    // Serialize the state to calculate rent/account size adjustments
-    let serialized = accs.attestation_state.1.try_to_vec()?;
-
-    if accs.attestation_state.is_initialized() {
-        accs.attestation_state
-            .info()
-            .realloc(serialized.len(), false)?;
-        trace!("Attestation state resize OK");
-
-        let target_rent = CreationLamports::Exempt.amount(serialized.len());
-        let current_rent = accs.attestation_state.info().lamports();
-
-        // Adjust rent, but only if there isn't enough
-        if target_rent > current_rent {
-            let transfer_amount = target_rent - current_rent;
-
-            let transfer_ix = system_instruction::transfer(
-                accs.payer.info().key,
-                accs.attestation_state.info().key,
-                transfer_amount,
-            );
-
-            invoke(&transfer_ix, ctx.accounts)?;
-        }
-
-        trace!("Attestation state rent transfer OK");
-    } else {
-        let seeds = accs
-            .attestation_state
-            .self_bumped_seeds(None, ctx.program_id);
-        solitaire::create_account(
-            ctx,
-            accs.attestation_state.info(),
-            accs.payer.key,
-            solitaire::CreationLamports::Exempt,
-            serialized.len(),
-            ctx.program_id,
-            solitaire::IsSigned::SignedWithSeeds(&[seeds
-                .iter()
-                .map(|s| s.as_slice())
-                .collect::<Vec<_>>()
-                .as_slice()]),
-        )?;
-        trace!("Attestation state init OK");
-    }
     let bridge_config = BridgeData::try_from_slice(&accs.wh_bridge.try_borrow_mut_data()?)?.config;
 
     // Pay wormhole fee

wormhole_attester/program/src/attestation_state.rs

@@ -14,33 +14,47 @@ use {
         AccountOwner,
         AccountState,
         Data,
-        Derive,
         Owned,
+        Peel,
+        Seeded,
     },
-    std::collections::BTreeMap,
 };
 
 /// On-chain state for a single price attestation
-#[derive(BorshSerialize, BorshDeserialize)]
+#[derive(BorshSerialize, BorshDeserialize, Default)]
 pub struct AttestationState {
     /// The last trading publish_time this attester saw
     pub last_attested_trading_publish_time: UnixTimestamp,
 }
 
-/// Top-level state gathering all known AttestationState values, keyed by price address.
-#[derive(BorshSerialize, BorshDeserialize, Default)]
-pub struct AttestationStateMap {
-    pub entries: BTreeMap<Pubkey, AttestationState>,
+impl Owned for AttestationState {
+    fn owner(&self) -> AccountOwner {
+        AccountOwner::This
+    }
 }
 
+pub struct AttestationStatePDA<'b>(
+    pub Data<'b, AttestationState, { AccountState::MaybeInitialized }>,
+);
 
-impl Owned for AttestationStateMap {
-    fn owner(&self) -> AccountOwner {
-        AccountOwner::This
+impl Seeded<&Pubkey> for AttestationStatePDA<'_> {
+    fn seeds(symbol_id: &Pubkey) -> Vec<Vec<u8>> {
+        vec![
+            "p2w-attestation-state-v1".as_bytes().to_vec(),
+            symbol_id.to_bytes().to_vec(),
+        ]
     }
 }
 
-pub type AttestationStateMapPDA<'b> = Derive<
-    Data<'b, AttestationStateMap, { AccountState::MaybeInitialized }>,
-    "p2w-attestation-state-v1",
->;
+impl<'a, 'b: 'a> Peel<'a, 'b> for AttestationStatePDA<'b> {
+    fn peel<I>(ctx: &mut solitaire::Context<'a, 'b, I>) -> solitaire::Result<Self>
+    where
+        Self: Sized,
+    {
+        Ok(Self(Data::peel(ctx)?))
+    }
+
+    fn persist(&self, program_id: &Pubkey) -> solitaire::Result<()> {
+        self.0.persist(program_id)
+    }
+}

wormhole_attester/sdk/rust/src/lib.rs

@@ -519,7 +519,7 @@ mod tests {
             status:                     PriceStatus::Trading,
             num_publishers:             123212u32,
             max_num_publishers:         321232u32,
-            attestation_time:           (0xdeadbeeffadeu64) as i64,
+            attestation_time:           (0xdeadbeeffadedeedu64) as i64,
             publish_time:               0xdadebeefi64,
             prev_publish_time:          0xdeadbabei64,
             prev_price:                 0xdeadfacebeefi64,