Add batch price update instruction to Oracle (#142)

* Remove unused function rpc::upd_price::build_tx. The batched version is used everywhere in the codebase.

* Capture solana logs in integration tests

* Add cmd_upd_price_no_fail_on_error

* Change RPC client to send e_cmd_upd_price_no_fail_on_error instructions

Author: tompntn

pc/rpc_client.cpp

@@ -809,7 +809,7 @@ rpc::upd_price::upd_price()
   ckey_( nullptr ),
   gkey_( nullptr ),
   akey_( nullptr ),
-  cmd_( e_cmd_upd_price )
+  cmd_( e_cmd_upd_price_no_fail_on_error )
 {
 }
 
@@ -851,7 +851,7 @@ void rpc::upd_price::set_price( int64_t px,
   price_ = px;
   conf_  = conf;
   st_    = st;
-  cmd_   = is_agg?e_cmd_agg_price:e_cmd_upd_price;
+  cmd_   = is_agg?e_cmd_agg_price:e_cmd_upd_price_no_fail_on_error;
 }
 
 void rpc::upd_price::set_slot( const uint64_t pub_slot )
@@ -884,52 +884,6 @@ class tx_wtr : public net_wtr
   }
 };
 
-void rpc::upd_price::build_tx( bincode& tx )
-{
-  // signatures section
-  tx.add_len<1>();      // one signature (publish)
-  size_t pub_idx = tx.reserve_sign();
-
-  // message header
-  size_t tx_idx = tx.get_pos();
-  tx.add( (uint8_t)1 ); // pub is only signing account
-  tx.add( (uint8_t)0 ); // read-only signed accounts
-  tx.add( (uint8_t)2 ); // sysvar and program-id are read-only
-                        // unsigned accounts
-
-  // accounts
-  tx.add_len<4>();      // 4 accounts: publish, symbol, sysvar, program
-  tx.add( *pkey_ );     // publish account
-  tx.add( *akey_ );     // symbol account
-  tx.add( *(pub_key*)sysvar_clock ); // sysvar account
-  tx.add( *gkey_ );     // programid
-
-  // recent block hash
-  tx.add( *bhash_ );    // recent block hash
-
-  // instructions section
-  tx.add_len<1>();      // one instruction
-  tx.add( (uint8_t)3);  // program_id index
-  tx.add_len<3>();      // 3 accounts: publish, symbol, sysvar
-  tx.add( (uint8_t)0 ); // index of publish account
-  tx.add( (uint8_t)1 ); // index of symbol account
-  tx.add( (uint8_t)2 ); // index of sysvar account
-
-  // instruction parameter section
-  tx.add_len<sizeof(cmd_upd_price)>();
-  tx.add( (uint32_t)PC_VERSION );
-  tx.add( (int32_t)cmd_ );
-  tx.add( (int32_t)st_ );
-  tx.add( (int32_t)0 );
-  tx.add( price_ );
-  tx.add( conf_ );
-  tx.add( pub_slot_ );
-
-  // all accounts need to sign transaction
-  tx.sign( pub_idx, tx_idx, *ckey_ );
-  sig_.init_from_buf( (const uint8_t*)(tx.get_buf() + pub_idx) );
-}
-
 bool rpc::upd_price::build_tx(
   bincode& tx, upd_price* upds[], const unsigned n
 )

pc/rpc_client.hpp

@@ -429,7 +429,6 @@ namespace pc
       static bool request( json_wtr&, upd_price*[], const unsigned n );
 
     private:
-      void build_tx( bincode& );
       static bool build_tx( bincode&, upd_price*[], unsigned n );
 
       hash         *bhash_;

program/src/oracle/oracle.c

@@ -520,7 +520,7 @@ static uint64_t upd_price( SolParameters *prm, SolAccountInfo *ka )
   // reject if this price corresponds to the same or earlier time
   pc_price_info_t *fptr = &pptr->comp_[i].latest_;
   sysvar_clock_t *sptr = (sysvar_clock_t*)ka[clock_idx].data;
-  if ( cptr->cmd_ == e_cmd_upd_price &&
+  if ( ( cptr->cmd_ == e_cmd_upd_price || cptr->cmd_ == e_cmd_upd_price_no_fail_on_error ) &&
        cptr->pub_slot_ <= fptr->pub_slot_ ) {
     return ERROR_INVALID_ARGUMENT;
   }
@@ -531,7 +531,7 @@ static uint64_t upd_price( SolParameters *prm, SolAccountInfo *ka )
   }
 
   // update component price if required
-  if ( cptr->cmd_ == e_cmd_upd_price ) {
+  if ( cptr->cmd_ == e_cmd_upd_price || cptr->cmd_ == e_cmd_upd_price_no_fail_on_error ) {
     fptr->price_    = cptr->price_;
     fptr->conf_     = cptr->conf_;
     fptr->status_   = cptr->status_;
@@ -540,6 +540,12 @@ static uint64_t upd_price( SolParameters *prm, SolAccountInfo *ka )
   return SUCCESS;
 }
 
+static uint64_t upd_price_no_fail_on_error( SolParameters *prm, SolAccountInfo *ka )
+{
+  upd_price( prm, ka );
+  return SUCCESS;
+}
+
 static uint64_t dispatch( SolParameters *prm, SolAccountInfo *ka )
 {
   if (prm->data_len < sizeof(cmd_hdr_t) ) {
@@ -551,19 +557,20 @@ static uint64_t dispatch( SolParameters *prm, SolAccountInfo *ka )
   }
   switch(hdr->cmd_) {
     case e_cmd_upd_price:
-    case e_cmd_agg_price:     return upd_price( prm, ka );
-    case e_cmd_init_mapping:  return init_mapping( prm, ka );
-    case e_cmd_add_mapping:   return add_mapping( prm, ka );
-    case e_cmd_add_product:   return add_product( prm, ka );
-    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 );
-    case e_cmd_del_publisher: return del_publisher( prm, ka );
-    case e_cmd_init_price:    return init_price( prm, ka );
-    case e_cmd_init_test:     return init_test( prm, ka );
-    case e_cmd_upd_test:      return upd_test( prm, ka );
-    case e_cmd_set_min_pub:   return set_min_pub( prm, ka );
-    default:                  return ERROR_INVALID_ARGUMENT;
+    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 );
+    case e_cmd_init_mapping:               return init_mapping( prm, ka );
+    case e_cmd_add_mapping:                return add_mapping( prm, ka );
+    case e_cmd_add_product:                return add_product( prm, ka );
+    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 );
+    case e_cmd_del_publisher:              return del_publisher( prm, ka );
+    case e_cmd_init_price:                 return init_price( prm, ka );
+    case e_cmd_init_test:                  return init_test( prm, ka );
+    case e_cmd_upd_test:                   return upd_test( prm, ka );
+    case e_cmd_set_min_pub:                return set_min_pub( prm, ka );
+    default:                               return ERROR_INVALID_ARGUMENT;
   }
 }
 

program/src/oracle/oracle.h

@@ -221,6 +221,12 @@ typedef enum {
   // key[2] sysvar_clock account  [readable]
   e_cmd_upd_price,
 
+  // publish component price, never returning an error even if the update failed
+  // key[0] funding account       [signer writable]
+  // key[1] price account         [writable]
+  // key[2] sysvar_clock account  [readable]
+  e_cmd_upd_price_no_fail_on_error,
+
   // compute aggregate price
   // key[0] funding account       [signer writable]
   // key[1] price account         [writable]

program/src/oracle/test_oracle.c

@@ -448,6 +448,103 @@ Test( oracle, upd_price ) {
   cr_assert( ERROR_INVALID_ARGUMENT == dispatch( &prm, acc ) );
 }
 
+Test( oracle, upd_price_no_fail_on_error ) {
+  cmd_upd_price_t idata = {
+    .ver_    = PC_VERSION,
+    .cmd_    = e_cmd_upd_price_no_fail_on_error,
+    .status_ = PC_STATUS_TRADING,
+    .price_  = 42L,
+    .conf_   = 9L,
+    .pub_slot_ = 1
+  };
+  SolPubkey p_id  = {.x = { 0xff, }};
+  SolPubkey pkey = {.x = { 1, }};
+  SolPubkey skey = {.x = { 3, }};
+  sysvar_clock_t cvar = {
+    .slot_ = 1
+  };
+  uint64_t pqty = 100, sqty = 200;
+  pc_price_t sptr[1];
+  sol_memset( sptr, 0, sizeof( pc_price_t ) );
+  sptr->magic_ = PC_MAGIC;
+  sptr->ver_   = PC_VERSION;
+  sptr->ptype_ = PC_PTYPE_PRICE;
+  sptr->type_  = PC_ACCTYPE_PRICE;
+  sptr->num_   = 1;
+  
+  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         = &skey,
+      .lamports    = &sqty,
+      .data_len    = sizeof( pc_price_t ),
+      .data        = (uint8_t*)sptr,
+      .owner       = &p_id,
+      .rent_epoch  = 0,
+      .is_signer   = false,
+      .is_writable = true,
+      .executable  = false
+  },{
+      .key         = (SolPubkey*)sysvar_clock,
+      .lamports    = &sqty,
+      .data_len    = sizeof( sysvar_clock_t ),
+      .data        = (uint8_t*)&cvar,
+      .owner       = &p_id,
+      .rent_epoch  = 0,
+      .is_signer   = false,
+      .is_writable = false,
+      .executable  = false
+  }};
+  SolParameters prm = {
+    .ka         = acc,
+    .ka_num     = 3,
+    .data       = (const uint8_t*)&idata,
+    .data_len   = sizeof( idata ),
+    .program_id = &p_id
+  };
+
+  // We haven't permissioned the publish account for the price account
+  // yet, so any update should fail silently and have no effect. The
+  // transaction should "succeed".
+  cr_assert( SUCCESS == dispatch( &prm, acc ) );
+  cr_assert( sptr->comp_[0].latest_.price_ == 0L );
+  cr_assert( sptr->comp_[0].latest_.conf_ == 0L );
+  cr_assert( sptr->comp_[0].latest_.pub_slot_ == 0 );
+  cr_assert( sptr->agg_.pub_slot_ == 0 );
+  cr_assert( sptr->valid_slot_ == 0 );  
+
+  // Now permission the publish account for the price account.
+  pc_pub_key_assign( &sptr->comp_[0].pub_, (pc_pub_key_t*)&pkey );
+
+  // The update should now succeed, and have an effect.
+  cr_assert( SUCCESS == dispatch( &prm, acc ) );
+  cr_assert( sptr->comp_[0].latest_.price_ == 42L );
+  cr_assert( sptr->comp_[0].latest_.conf_ == 9L );
+  cr_assert( sptr->comp_[0].latest_.pub_slot_ == 1 );
+  cr_assert( sptr->agg_.pub_slot_ == 1 );
+  cr_assert( sptr->valid_slot_ == 0 );
+
+  // Invalid updates, such as publishing an update for the current slot, 
+  // should still fail silently and have no effect.
+  idata.price_ = 55L;
+  idata.conf_ = 22L;
+  idata.pub_slot_ = 1;
+  cr_assert( SUCCESS == dispatch( &prm, acc ) );
+  cr_assert( sptr->comp_[0].latest_.price_ == 42L );
+  cr_assert( sptr->comp_[0].latest_.conf_ == 9L );
+  cr_assert( sptr->comp_[0].latest_.pub_slot_ == 1 );
+  cr_assert( sptr->agg_.pub_slot_ == 1 );
+  cr_assert( sptr->valid_slot_ == 0 );
+}
+
 Test( oracle, upd_aggregate ) {
   pc_price_t px[1];
   sol_memset( px, 0, sizeof( pc_price_t ) );

pyth/tests/conftest.py

@@ -321,6 +321,19 @@ def pyth_add_publisher(
     return pyth_add_price
 
 
+@pytest.fixture(scope='session')
+def solana_logs(solana_test_validator, solana_keygen):
+    with open("solana_logs.txt", 'w') as f:
+        cmd = [
+            'solana', 'logs',
+            '--url', 'localhost',
+            '--keypair', solana_keygen[1],
+        ]
+        p = subprocess.Popen(cmd, stdout=f)
+        yield
+        p.kill()
+
+
 @pytest.fixture(scope='function')
 def pyth_init_price(solana_test_validator, pyth_dir, pyth_add_publisher):
 

pyth/tests/test_update_price.py

@@ -9,7 +9,7 @@
 from pyth.tests.conftest import PRODUCTS
 
 @pytest.mark.asyncio
-async def test_batch_update_price(solana_test_validator, pythd, pyth_dir, pyth_init_product, pyth_init_price):
+async def test_batch_update_price(solana_test_validator, solana_logs, pythd, pyth_dir, pyth_init_product, pyth_init_price):
 
     messageIds = itertools.count()