Skip to content

Commit

Permalink
refactor: remove mapping account usage (#145)
Browse files Browse the repository at this point in the history
Solana get program accounts RPC call is a great way to track price and product accounts instead of mapping account. Mapping account is not efficient and gets in the way if we change its structure. This PR removes all the usages of the mapping account and cleans up some legacy tests/codes in the code.
  • Loading branch information
ali-bahjati authored Sep 17, 2024
1 parent 557a79e commit 1179739
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 454 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
[package]
name = "pyth-agent"
version = "2.11.1"
version = "2.12.0"
edition = "2021"

[[bin]]
name = "agent"
path = "src/bin/agent.rs"

[[bin]]
name = "agent-migrate-config"
path = "src/bin/agent_migrate_config.rs"

[dependencies]
anyhow = "1.0.81"
serde = { version = "1.0.197", features = ["derive"] }
Expand Down
3 changes: 0 additions & 3 deletions config/config.sample.pythnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ key_store.publish_keypair_path = "/path/to/keypair.json"
# Oracle program pubkey
key_store.program_key = "FsJ3A3u2vn5cTVofAjvy6y5kwABJAqYWpe4975bi2epH"

# Oracle mapping pubkey
key_store.mapping_key = "AHtgzX45WTKfkPG53L6WYhGEXwQkN1BVknET3sVsLL8J"

# Compute unit per price update.
exporter.compute_unit_limit = 5000

Expand Down
4 changes: 0 additions & 4 deletions config/config.sample.pythtest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ key_store.publish_keypair_path = "/path/to/keypair.json"
key_store.program_key = "8tfDNiaEyrV6Q1U4DEXrEigs9DoDtkugzFbybENEbCDz" # conformance
# key_store.program_key = "gSbePebfvPy7tRqimPoVecS2UsBvYv46ynrzWocc92s" # cross-chain

# Oracle mapping pubkey
key_store.mapping_key = "AFmdnt9ng1uVxqCmqwQJDAYC5cKTkw8gJKSM5PnzuF6z" # conformance
# key_store.mapping_key = "BmA9Z6FjioHJPpjT39QazZyhDRUdZy2ezwx4GiDdE2u2" # cross-chain

# Pythtest accumulator key (only for the cross-chain oracle)
# key_store.accumulator_key = "7Vbmv1jt4vyuqBZcpYPpnVhrqVe5e6ZPb6JxDcffRHUM"

Expand Down
3 changes: 0 additions & 3 deletions config/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ key_store.publish_keypair_path = "/path/to/keypair.json"
# Public key of the oracle program
key_store.program_key = "RelevantOracleProgramAddress"

# Public key of the root mapping account
key_store.mapping_key = "RelevantOracleMappingAddress"

### Optional fields of primary/secondary network config ###

# Pubkey of accumulator message buffer program ID. Setting this
Expand Down
107 changes: 12 additions & 95 deletions integration-tests/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@
from contextlib import contextmanager
import shutil
from solana.keypair import Keypair
from solders.system_program import ID as SYSTEM_PROGRAM_ID
from solana.rpc.async_api import AsyncClient
from solana.rpc import commitment
from solana.transaction import AccountMeta, Transaction, TransactionInstruction
from solana.transaction import AccountMeta, Transaction
from anchorpy import Provider, Wallet
from construct import Bytes, Int32sl, Int32ul, Struct
from solana.publickey import PublicKey
from message_buffer_client_codegen.instructions import initialize, set_allowed_programs, create_buffer
from message_buffer_client_codegen.accounts.message_buffer import MessageBuffer
Expand Down Expand Up @@ -359,21 +357,6 @@ def agent_publish_keypair(self, agent_keystore_path, sync_accounts):
LOGGER.debug(f"Publisher {address.stdout.strip()} balance: {balance.stdout.strip()}")
time.sleep(8)

@pytest.fixture
def agent_keystore(self, agent_keystore_path, agent_publish_keypair):
self.run(
f"../scripts/init_key_store.sh localnet {agent_keystore_path}")

if USE_ACCUMULATOR:
path = os.path.join(agent_keystore_path, "accumulator_program_key.json")

with open(path, 'w') as f:
f.write(MESSAGE_BUFFER_PROGRAM)

if os.path.exists("keystore"):
os.remove("keystore")
os.symlink(agent_keystore_path, "keystore")

@pytest_asyncio.fixture
async def initialize_message_buffer_program(self, funding_keypair, sync_key_path, sync_accounts):

Expand Down Expand Up @@ -429,18 +412,15 @@ async def initialize_message_buffer_program(self, funding_keypair, sync_key_path
await provider.send(tx, [parsed_funding_keypair])

@pytest.fixture
def agent_config(self, agent_keystore, agent_keystore_path, tmp_path):
def agent_config(self, agent_keystore_path, agent_publish_keypair, tmp_path):
with open("agent_conf.toml") as config_file:
agent_config = config_file.read()

publish_keypair_path = os.path.join(agent_keystore_path, "publish_key_pair.json")

mapping_keypair = Keypair.from_secret_key(MAPPING_KEYPAIR)

agent_config += f"""
key_store.publish_keypair_path = "{publish_keypair_path}"
key_store.program_key = "{ORACLE_PROGRAM}"
key_store.mapping_key = "{mapping_keypair.public_key}"
"""

# Add accumulator setting if option is enabled
Expand All @@ -457,32 +437,7 @@ def agent_config(self, agent_keystore, agent_keystore_path, tmp_path):
return path

@pytest.fixture
def agent_legacy_config(self, agent_keystore, agent_keystore_path, tmp_path):
"""
Prepares a legacy v1.x.x config for testing agent-migrate-config
"""
with open("agent_conf.toml") as config_file:
agent_config = config_file.read()

agent_config += f'\nkey_store.root_path = "{agent_keystore_path}"'

if USE_ACCUMULATOR:
# Add accumulator setting to verify that it is inlined as well
agent_config += f'\nkey_store.accumulator_key_path = "accumulator_program_key.json"'

LOGGER.debug(f"Built legacy agent config:\n{agent_config}")

path = os.path.join(tmp_path, "agent_conf_legacy.toml")

with open(path, 'w') as f:
f.write(agent_config)

return path



@pytest.fixture
def agent(self, sync_accounts, agent_keystore, tmp_path, initialize_message_buffer_program, agent_config):
def agent(self, sync_accounts, agent_keystore_path, agent_publish_keypair, tmp_path, initialize_message_buffer_program, agent_config):
LOGGER.debug("Building agent binary")
self.run("cargo build --release --bin agent")

Expand All @@ -496,7 +451,7 @@ def agent(self, sync_accounts, agent_keystore, tmp_path, initialize_message_buff
yield

@pytest.fixture
def agent_hotload(self, sync_accounts, agent_keystore, agent_keystore_path, tmp_path, initialize_message_buffer_program, agent_config):
def agent_hotload(self, sync_accounts, agent_keystore_path, tmp_path, initialize_message_buffer_program, agent_config):
"""
Spawns an agent without a publish keypair, used for keypair hotloading testing
"""
Expand Down Expand Up @@ -560,11 +515,11 @@ async def test_update_price_simple(self, client: PythAgentClient):

# Send an "update_price" request
await client.update_price(price_account, 42, 2, "trading")
time.sleep(2)
time.sleep(5)

# Send another "update_price" request to trigger aggregation
await client.update_price(price_account, 81, 1, "trading")
time.sleep(2)
time.sleep(5)

# Confirm that the price account has been updated with the values from the first "update_price" request
final_product_state = await client.get_product(product_account)
Expand Down Expand Up @@ -726,44 +681,6 @@ async def test_publish_forever(self, client: PythAgentClient, tmp_path):
await client.update_price(price_account, 47, 2, "trading")
time.sleep(1)

@pytest.mark.asyncio
async def test_agent_migrate_config(self,
agent_keystore,
agent_legacy_config,
agent_migrate_config_binary,
client_no_spawn: PythAgentClient,
initialize_message_buffer_program,
sync_accounts,
tmp_path,
):
os.environ["RUST_BACKTRACE"] = "full"
os.environ["RUST_LOG"] = "debug"

# Migrator must run successfully (run() raises on error)
new_config = self.run(f"{agent_migrate_config_binary} -c {agent_legacy_config}").stdout.strip()

LOGGER.debug(f"Successfully migrated legacy config to:\n{new_config}")

# Overwrite legacy config with the migrated version.
#
# NOTE: assumes 'w' erases the file before access)
with open(agent_legacy_config, 'w') as f:
f.write(new_config)
f.flush()

self.run("cargo build --release --bin agent")

log_dir = os.path.join(tmp_path, "agent_logs")

# We start the agent manually to pass it the updated legacy config
with self.spawn(f"../target/release/agent --config {agent_legacy_config}", log_dir=log_dir):
time.sleep(3)
await client_no_spawn.connect()

# Continue with the simple test case, which must succeed
await self.test_update_price_simple(client_no_spawn)
await client_no_spawn.close()

@pytest.mark.asyncio
async def test_agent_respects_market_hours(self, client: PythAgentClient):
'''
Expand All @@ -784,13 +701,13 @@ async def test_agent_respects_market_hours(self, client: PythAgentClient):

# Send an "update_price" request
await client.update_price(price_account, 42, 2, "trading")
time.sleep(2)
time.sleep(5)

# Send another update_price request to "trigger" aggregation
# (aggregation would happen if market hours were to fail, but
# we want to catch that happening if there's a problem)
await client.update_price(price_account, 81, 1, "trading")
time.sleep(2)
time.sleep(5)

# Confirm that the price account has not been updated
final_product_state = await client.get_product(product_account)
Expand Down Expand Up @@ -819,13 +736,13 @@ async def test_agent_respects_holiday_hours(self, client: PythAgentClient):

# Send an "update_price" request
await client.update_price(price_account, 42, 2, "trading")
time.sleep(2)
time.sleep(5)

# Send another update_price request to "trigger" aggregation
# (aggregation would happen if market hours were to fail, but
# we want to catch that happening if there's a problem)
await client.update_price(price_account, 81, 1, "trading")
time.sleep(2)
time.sleep(5)

# Confirm that the price account has not been updated
final_product_state = await client.get_product(product_account)
Expand Down Expand Up @@ -861,7 +778,7 @@ async def test_agent_respects_publish_interval(self, client: PythAgentClient):
# (aggregation would happen if publish interval were to fail, but
# we want to catch that happening if there's a problem)
await client.update_price(price_account, 81, 1, "trading")
time.sleep(2)
time.sleep(5)

# Confirm that the price account has not been updated
final_product_state = await client.get_product(product_account)
Expand All @@ -875,7 +792,7 @@ async def test_agent_respects_publish_interval(self, client: PythAgentClient):
# Send another update_price request to "trigger" aggregation
# Now it is after the publish interval, so the price should be updated
await client.update_price(price_account, 81, 1, "trading")
time.sleep(2)
time.sleep(5)

# Confirm that the price account has been updated
final_product_state = await client.get_product(product_account)
Expand Down
45 changes: 0 additions & 45 deletions scripts/init_key_store.sh

This file was deleted.

8 changes: 4 additions & 4 deletions src/agent/services/oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ where
config.clone(),
network,
state.clone(),
key_store.mapping_key,
key_store.pyth_oracle_program_key,
key_store.publish_keypair,
key_store.pyth_price_store_program_key,
config.oracle.max_lookup_batch_size,
Expand Down Expand Up @@ -152,13 +152,13 @@ where
Ok(())
}

/// On poll lookup all Pyth Mapping/Product/Price accounts and sync.
/// On poll lookup all Pyth Product/Price accounts and sync.
#[instrument(skip(config, publish_keypair, state))]
async fn poller<S>(
config: Config,
network: Network,
state: Arc<S>,
mapping_key: Pubkey,
oracle_program_key: Pubkey,
publish_keypair: Option<Keypair>,
pyth_price_store_program_key: Option<Pubkey>,
max_lookup_batch_size: usize,
Expand All @@ -183,7 +183,7 @@ async fn poller<S>(
Oracle::poll_updates(
&*state,
network,
mapping_key,
oracle_program_key,
publish_keypair.as_ref(),
pyth_price_store_program_key,
&client,
Expand Down
9 changes: 0 additions & 9 deletions src/agent/solana.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,6 @@ pub mod key_store {
default
)]
pub pyth_price_store_program_key: Option<Pubkey>,
/// The public key of the root mapping account
#[serde(
serialize_with = "pubkey_string_ser",
deserialize_with = "pubkey_string_de"
)]
pub mapping_key: Pubkey,
/// The public key of the accumulator program.
#[serde(
serialize_with = "opt_pubkey_string_ser",
Expand All @@ -127,8 +121,6 @@ pub mod key_store {
pub pyth_oracle_program_key: Pubkey,
/// Public key of the pyth-price-store program
pub pyth_price_store_program_key: Option<Pubkey>,
/// Public key of the root mapping account
pub mapping_key: Pubkey,
/// Public key of the accumulator program (if provided)
pub accumulator_key: Option<Pubkey>,
}
Expand All @@ -151,7 +143,6 @@ pub mod key_store {
publish_keypair,
pyth_oracle_program_key: config.pyth_oracle_program_key,
pyth_price_store_program_key: config.pyth_price_store_program_key,
mapping_key: config.mapping_key,
accumulator_key: config.accumulator_key,
})
}
Expand Down
Loading

0 comments on commit 1179739

Please sign in to comment.