Implement add product instruction (#225)

* initial version

* cleaning up stuff

* add tests

* delete c impl

* pr comments

* delete C test

* one more

Co-authored-by: Jayant Krishnamurthy <jkrishnamurthy@jumptrading.com>

Author: jayantk

program/c/src/oracle/oracle.c

@@ -59,48 +59,6 @@ static bool valid_writable_account( SolParameters *prm,
          is_rent_exempt( *ka->lamports, ka->data_len );
 }
 
-static uint64_t add_product( SolParameters *prm, SolAccountInfo *ka )
-{
-  // Account (1) is the mapping account that we're going to add to and
-  // must be the tail (or last) mapping account in chain
-  // Account (2) is the new product account
-  // Verify that these are signed, writable accounts with correct ownership
-  // and size
-  if ( prm->ka_num != 3 ||
-       !valid_funding_account( &ka[0] ) ||
-       !valid_signable_account( prm, &ka[1], sizeof( pc_map_table_t ) ) ||
-       !valid_signable_account( prm, &ka[2], PC_PROD_ACC_SIZE ) ) {
-    return ERROR_INVALID_ARGUMENT;
-  }
-
-  // Verify that mapping account is a valid tail account
-  // that the new product account is uninitialized and that there is space
-  // in the mapping account
-  cmd_hdr_t *hdr = (cmd_hdr_t*)prm->data;
-  pc_map_table_t *mptr = (pc_map_table_t*)ka[1].data;
-  pc_prod_t *pptr = (pc_prod_t*)ka[2].data;
-  if ( mptr->magic_ != PC_MAGIC ||
-       mptr->ver_ != hdr->ver_ ||
-       mptr->type_ != PC_ACCTYPE_MAPPING ||
-       pptr->magic_ != 0 ||
-       mptr->num_ >= PC_MAP_TABLE_SIZE ) {
-    return ERROR_INVALID_ARGUMENT;
-  }
-
-  // Initialize product account
-  sol_memset( pptr, 0, PC_PROD_ACC_SIZE );
-  pptr->magic_ = PC_MAGIC;
-  pptr->ver_   = hdr->ver_;
-  pptr->type_  = PC_ACCTYPE_PRODUCT;
-  pptr->size_  = sizeof( pc_prod_t );
-
-  // finally add mapping account link
-  pc_pub_key_assign( &mptr->prod_[mptr->num_++], (pc_pub_key_t*)ka[2].key );
-  mptr->size_  = sizeof( pc_map_table_t ) - sizeof( mptr->prod_ ) +
-    mptr->num_ * sizeof( pc_pub_key_t );
-  return SUCCESS;
-}
-
 #define PC_ADD_STR \
   tag = (pc_str_t*)src;\
   tag_len = 1 + tag->len_;\
@@ -471,10 +429,9 @@ static uint64_t dispatch( SolParameters *prm, SolAccountInfo *ka )
     case e_cmd_upd_price:
     case e_cmd_agg_price:                  return upd_price( prm, ka );
     case e_cmd_upd_price_no_fail_on_error: return upd_price_no_fail_on_error( prm, ka );
-    // init_mapping is overridden in Rust, but still implemented here to make the C unit tests pass.
     case e_cmd_init_mapping:               return ERROR_INVALID_ARGUMENT;
     case e_cmd_add_mapping:                return ERROR_INVALID_ARGUMENT;
-    case e_cmd_add_product:                return add_product( prm, ka );
+    case e_cmd_add_product:                return ERROR_INVALID_ARGUMENT;
     case e_cmd_upd_product:                return upd_product( prm, ka );
     case e_cmd_add_price:                  return add_price( prm, ka );
     case e_cmd_add_publisher:              return add_publisher( prm, ka );

program/c/src/oracle/test_oracle.c

@@ -7,101 +7,6 @@ uint64_t MAPPING_ACCOUNT_LAMPORTS = 143821440;
 uint64_t PRODUCT_ACCOUNT_LAMPORTS = 4454400;
 uint64_t PRICE_ACCOUNT_LAMPORTS = 23942400;
 
-Test(oracle, add_product) {
-  // start with perfect inputs
-  cmd_add_product_t idata = {
-    .ver_   = PC_VERSION,
-    .cmd_   = e_cmd_add_product,
-  };
-  SolPubkey p_id  = {.x = { 0xff, }};
-  SolPubkey pkey = {.x = { 1, }};
-  SolPubkey mkey = {.x = { 2, }};
-  SolPubkey skey = {.x = { 3, }};
-  SolPubkey skey2 = {.x = { 4, }};
-  uint64_t pqty = 100;
-  pc_map_table_t mptr[1];
-  sol_memset( mptr, 0, sizeof( pc_map_table_t ) );
-  mptr->magic_ = PC_MAGIC;
-  mptr->ver_ = PC_VERSION;
-  mptr->type_ = PC_ACCTYPE_MAPPING;
-  char sdata[PC_PROD_ACC_SIZE];
-  pc_prod_t *sptr = (pc_prod_t*)sdata;
-  sol_memset( sptr, 0, PC_PROD_ACC_SIZE );
-  SolAccountInfo acc[] = {{
-      .key         = &pkey,
-      .lamports    = &pqty,
-      .data_len    = 0,
-      .data        = NULL,
-      .owner       = NULL,
-      .rent_epoch  = 0,
-      .is_signer   = true,
-      .is_writable = true,
-      .executable  = false
-  },{
-      .key         = &mkey,
-      .lamports    = &MAPPING_ACCOUNT_LAMPORTS,
-      .data_len    = sizeof( pc_map_table_t ),
-      .data        = (uint8_t*)mptr,
-      .owner       = &p_id,
-      .rent_epoch  = 0,
-      .is_signer   = true,
-      .is_writable = true,
-      .executable  = false
-  },{
-      .key         = &skey,
-      .lamports    = &PRODUCT_ACCOUNT_LAMPORTS,
-      .data_len    = PC_PROD_ACC_SIZE,
-      .data        = (uint8_t*)sptr,
-      .owner       = &p_id,
-      .rent_epoch  = 0,
-      .is_signer   = true,
-      .is_writable = true,
-      .executable  = false
-  }};
-  SolParameters prm = {
-    .ka         = acc,
-    .ka_num     = 3,
-    .data       = (const uint8_t*)&idata,
-    .data_len   = sizeof( idata ),
-    .program_id = &p_id
-  };
-  cr_assert( SUCCESS == dispatch( &prm, acc ) );
-  cr_assert( sptr->magic_ == PC_MAGIC );
-  cr_assert( sptr->ver_ == PC_VERSION );
-  cr_assert( sptr->type_ == PC_ACCTYPE_PRODUCT );
-  cr_assert( sptr->size_ == sizeof( pc_prod_t ) );
-  cr_assert( mptr->num_ == 1 );
-  cr_assert( pc_pub_key_equal( (pc_pub_key_t*)&skey, &mptr->prod_[0] ) );
-
-  // 2nd product
-  acc[2].key = &skey2;
-  sol_memset( sptr, 0, PC_PROD_ACC_SIZE );
-  cr_assert( SUCCESS == dispatch( &prm, acc ) );
-  cr_assert( mptr->num_ == 2 );
-  cr_assert( pc_pub_key_equal( &mptr->prod_[1], (pc_pub_key_t*)&skey2 ) );
-
-  // invalid acc size
-  acc[2].data_len = 1;
-  cr_assert(  ERROR_INVALID_ARGUMENT== dispatch( &prm, acc ) );
-  acc[2].data_len = PC_PROD_ACC_SIZE;
-
-  // test fill up of mapping table
-  sol_memset( mptr, 0, sizeof( pc_map_table_t ) );
-  mptr->magic_ = PC_MAGIC;
-  mptr->ver_ = PC_VERSION;
-  mptr->type_ = PC_ACCTYPE_MAPPING;
-  for( unsigned i = 0;; ++i ) {
-    sol_memset( sptr, 0, PC_PROD_ACC_SIZE );
-    uint64_t rc = dispatch( &prm, acc );
-    if ( rc != SUCCESS ) {
-      cr_assert( i == ( unsigned )(PC_MAP_TABLE_SIZE) );
-      break;
-    }
-    cr_assert( mptr->num_ == i + 1 );
-    cr_assert( rc == SUCCESS );
-  }
-}
-
 Test( oracle, add_publisher ) {
   // start with perfect inputs
   cmd_add_publisher_t idata = {

program/rust/src/processor.rs

@@ -9,6 +9,7 @@ use crate::c_oracle_header::{
     cmd_hdr,
     command_t_e_cmd_add_mapping,
     command_t_e_cmd_add_price,
+    command_t_e_cmd_add_product,
     command_t_e_cmd_agg_price,
     command_t_e_cmd_init_mapping,
     command_t_e_cmd_upd_account_version,
@@ -23,6 +24,7 @@ use crate::error::{
 use crate::rust_oracle::{
     add_mapping,
     add_price,
+    add_product,
     init_mapping,
     update_price,
     update_version,
@@ -63,6 +65,7 @@ pub fn process_instruction(
         command_t_e_cmd_add_price => add_price(program_id, accounts, instruction_data),
         command_t_e_cmd_init_mapping => init_mapping(program_id, accounts, instruction_data),
         command_t_e_cmd_add_mapping => add_mapping(program_id, accounts, instruction_data),
+        command_t_e_cmd_add_product => add_product(program_id, accounts, instruction_data),
         _ => c_entrypoint_wrapper(input),
     }
 }

program/rust/src/rust_oracle.rs

@@ -38,6 +38,7 @@ use crate::c_oracle_header::{
     PC_PTYPE_UNKNOWN,
 };
 use crate::error::OracleResult;
+use crate::OracleError;
 
 use crate::utils::pyth_assert;
 
@@ -171,6 +172,47 @@ pub fn add_price(
     Ok(SUCCESS)
 }
 
+pub fn add_product(
+    program_id: &Pubkey,
+    accounts: &[AccountInfo],
+    instruction_data: &[u8],
+) -> OracleResult {
+    let [_funding_account, tail_mapping_account, new_product_account] = match accounts {
+        [x, y, z]
+            if valid_funding_account(x)
+                && valid_signable_account(program_id, y, size_of::<pc_map_table_t>())
+                && valid_signable_account(program_id, z, PC_PROD_ACC_SIZE as usize)
+                && valid_fresh_account(z) =>
+        {
+            Ok([x, y, z])
+        }
+        _ => Err(ProgramError::InvalidArgument),
+    }?;
+
+    let hdr = load::<cmd_hdr_t>(instruction_data)?;
+    let mut mapping_data = load_mapping_account_mut(tail_mapping_account, hdr.ver_)?;
+    // The mapping account must have free space to add the product account
+    pyth_assert(
+        mapping_data.num_ < PC_MAP_TABLE_SIZE,
+        ProgramError::InvalidArgument,
+    )?;
+
+    initialize_product_account(new_product_account, hdr.ver_)?;
+
+    let current_index: usize = try_convert(mapping_data.num_)?;
+    unsafe {
+        mapping_data.prod_[current_index]
+            .k1_
+            .copy_from_slice(&new_product_account.key.to_bytes())
+    }
+    mapping_data.num_ += 1;
+    mapping_data.size_ =
+        try_convert::<_, u32>(size_of::<pc_map_table_t>() - size_of_val(&mapping_data.prod_))?
+            + mapping_data.num_ * try_convert::<_, u32>(size_of::<pc_pub_key_t>())?;
+
+    Ok(SUCCESS)
+}
+
 fn valid_funding_account(account: &AccountInfo) -> bool {
     account.is_signer && account.is_writable
 }
@@ -238,10 +280,24 @@ pub fn initialize_mapping_account(account: &AccountInfo, version: u32) -> Result
     Ok(())
 }
 
+/// Initialize account as a new product account. This function will zero out any existing data in
+/// the account.
+pub fn initialize_product_account(account: &AccountInfo, version: u32) -> Result<(), ProgramError> {
+    clear_account(account)?;
+
+    let mut prod_account = load_account_as_mut::<pc_prod_t>(account)?;
+    prod_account.magic_ = PC_MAGIC;
+    prod_account.ver_ = version;
+    prod_account.type_ = PC_ACCTYPE_PRODUCT;
+    prod_account.size_ = try_convert(size_of::<pc_prod_t>())?;
+
+    Ok(())
+}
+
 /// Mutably borrow the data in `account` as a product account, validating that the account
 /// is properly formatted. Any mutations to the returned value will be reflected in the
 /// account data. Use this to read already-initialized accounts.
-fn load_product_account_mut<'a>(
+pub fn load_product_account_mut<'a>(
     account: &'a AccountInfo,
     expected_version: u32,
 ) -> Result<RefMut<'a, pc_prod_t>, ProgramError> {
@@ -261,3 +317,9 @@ fn load_product_account_mut<'a>(
 pub fn pubkey_assign(target: &mut pc_pub_key_t, source: &[u8]) {
     unsafe { target.k1_.copy_from_slice(source) }
 }
+
+/// Convert `x: T` into a `U`, returning the appropriate `OracleError` if the conversion fails.
+fn try_convert<T, U: TryFrom<T>>(x: T) -> Result<U, OracleError> {
+    // Note: the error here assumes we're only applying this function to integers right now.
+    U::try_from(x).map_err(|_| OracleError::IntegerCastingError)
+}

program/rust/src/tests/mod.rs

@@ -1,2 +1,3 @@
 mod test_add_mapping;
+mod test_add_product;
 mod test_init_mapping;

program/rust/src/tests/test_add_mapping.rs

@@ -6,11 +6,11 @@ use crate::c_oracle_header::{
     PC_MAP_TABLE_SIZE,
     PC_VERSION,
 };
+use crate::deserialize::load_account_as_mut;
 use crate::rust_oracle::{
     add_mapping,
     clear_account,
     initialize_mapping_account,
-    load_account_as_mut,
     load_mapping_account_mut,
     pubkey_assign,
 };