diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml new file mode 100644 index 000000000..844c44d68 --- /dev/null +++ b/.github/workflows/claude-review.yml @@ -0,0 +1,141 @@ +name: Perform a code review when a pull request is created. +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + +jobs: + claude: + # Skip drafts, and skip fork PRs: on `pull_request` they don't get the + # ANTHROPIC_API_KEY secret, so the review would only fail. + if: >- + !github.event.pull_request.draft && + github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + structured_output: ${{ steps.run_claude.outputs.structured_output }} + steps: + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 + with: + # Explicitly check out the PR's merge commit. + ref: refs/pull/${{ github.event.pull_request.number }}/merge + persist-credentials: false + + - name: Pre-fetch base and head refs for the PR + env: + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + # Pass GitHub expressions through env and quote shell expansions. + git fetch --no-tags origin \ + "$PR_BASE_REF" \ + "+refs/pull/$PR_NUMBER/head" + + # If you want Claude to build and run code, install any dependencies that + # need to be downloaded before the "Run Claude" step, and add the commands + # it may run to --allowedTools below (e.g. "Bash(npm test:*)"). Any tool + # not listed there is denied. + + - name: Run Claude + id: run_claude + uses: anthropics/claude-code-action@86d88e619d8e6caf07b5c3944dd14f441c533718 # v1.0.243 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + # Use this job's read-only token instead of the Claude GitHub App, + # which needs `id-token: write` and would get write access. + github_token: ${{ github.token }} + prompt: | + This is PR #${{ github.event.pull_request.number }} for ${{ github.repository }}. + + Review ONLY the changes introduced by the PR, so consider: + git log --oneline ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} + git diff ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} + + Suggest any improvements, potential bugs, or issues. + Be concise and specific in your feedback. + + Return your review in two parts: + - `summary`: a short Markdown overview of the PR and your overall assessment. + - `comments`: one entry per specific issue, anchored to the line it concerns. + `path` is relative to the repository root. `line` is the line number in + the PR head version of the file (${{ github.event.pull_request.head.sha }}), + and must be a line shown in the diff above. The working tree is the merge + commit, so its line numbers can differ; check them with + `git show ${{ github.event.pull_request.head.sha }}:`. + Put issues that don't belong to a single line in `summary` instead. + + Pull request title and body: + ---- + ${{ github.event.pull_request.title }} + ${{ github.event.pull_request.body }} + claude_args: | + --allowedTools "Read,Grep,Glob,Bash(git log:*),Bash(git diff:*),Bash(git show:*)" + --json-schema '{ + "type": "object", + "properties": { + "summary": { + "type": "string", + "description": "Overall review as Markdown, posted as the review body." + }, + "comments": { + "type": "array", + "description": "Inline comments, each anchored to one line of the PR diff.", + "items": { + "type": "object", + "properties": { + "path": { "type": "string", "description": "File path relative to the repository root." }, + "line": { "type": "integer", "minimum": 1, "description": "Line number in the PR head version of the file. Must be within the diff." }, + "body": { "type": "string", "description": "The comment, as Markdown." } + }, + "required": ["path", "line", "body"] + } + } + }, + "required": ["summary", "comments"] + }' + + post_feedback: + runs-on: ubuntu-latest + needs: claude + if: needs.claude.outputs.structured_output != '' + permissions: + issues: write + pull-requests: write + steps: + - name: Report Claude feedback + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + CLAUDE_STRUCTURED_OUTPUT: ${{ needs.claude.outputs.structured_output }} + with: + github-token: ${{ github.token }} + script: | + const { summary = '', comments = [] } = JSON.parse(process.env.CLAUDE_STRUCTURED_OUTPUT); + const pr = context.payload.pull_request; + if (!summary.trim() && comments.length === 0) { + core.info('Claude returned an empty review; nothing to post.'); + return; + } + + try { + await github.rest.pulls.createReview({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number, + commit_id: pr.head.sha, + event: 'COMMENT', + body: summary.trim() || 'See inline comments.', + comments: comments.map(({ path, line, body }) => ({ path, line, side: 'RIGHT', body })), + }); + } catch (error) { + // GitHub rejects the whole review if any comment points outside the + // diff. Fall back to a single PR comment so no feedback is lost. + core.warning(`Could not post inline review (${error.message}); posting a plain comment instead.`); + const details = comments.map(({ path, line, body }) => `**\`${path}:${line}\`**\n\n${body}`); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + body: [summary.trim(), ...details].filter(Boolean).join('\n\n---\n\n'), + }); + } diff --git a/crates/core/src/scenarios/README.md b/crates/core/src/scenarios/README.md index 970da4824..97d131a9a 100644 --- a/crates/core/src/scenarios/README.md +++ b/crates/core/src/scenarios/README.md @@ -115,15 +115,23 @@ templates: | `{ type: input, value: oracle }` | the pubkey given in that `input` property (`value_type: pubkey`, never written) | | `{ type: pubkey, value:
}` | a fixed account | | `{ type: pda, program_id, seeds }` | a derived account; seeds resolve against the override's values | +| `{ type: owned, owner: , account: }` | every local account an owner owns whose data starts with that account's discriminator in the keep-alive template's IDL, including accounts loaded later; never fetched | `$slot`, `$slot±N` and `$unix_s`/`ms`/`us`/`ns` in `values` are filled from the slot and its Clock time; other values are written as given. Keep-alive starts in the slot the importing override plays and rewrites its fields every slot, after that slot's overrides, until the network is reset. An override of one of those fields therefore -never takes effect. An account not yet in the surfnet is fetched from the remote first, unless it, -or its owner with its owned accounts, is marked offline. Keep-alive is kept in memory only: after a restart on a persistent database it stops -until an importing override plays again. +never takes effect. An override with `keepAlive: false` stops keep-alive for the account it writes +from that slot on, so the account ages as the clock moves on, for example to test a stale price. A +later override with `keepAlive: true`, the default, on that account starts it again, with every +import that rewrites it, including imports bound by other templates' overrides. An override with +`keepAlive: false` binds none of its template's imports, so on its own it starts no keep-alive, also +not for the other accounts those imports rewrite: add an override with `keepAlive: true` when they +must stay fresh. An override with no values writes nothing, so it neither starts nor stops +keep-alive. An account not yet in the surfnet is fetched from the remote first, unless it, or its +owner with its owned accounts, is marked offline. Keep-alive is kept in memory only: after a restart +on a persistent database it stops until an importing override plays again. ### Override Templates Directly using the `surfnet_registerScenario` endpoint requires building out a map of account keys that are specific to the schema of the account that is being written to. diff --git a/crates/core/src/scenarios/keep_alive.rs b/crates/core/src/scenarios/keep_alive.rs index 9ca123dac..9534dcefd 100644 --- a/crates/core/src/scenarios/keep_alive.rs +++ b/crates/core/src/scenarios/keep_alive.rs @@ -1,13 +1,19 @@ //! Keep-alive covers what a protocol's own off-chain actor rewrites every slot, such as a market //! maker republishing its quote. Nobody runs those actors against a surfnet, so once a scenario's //! override touches an account, the surfnet applies the keep-alive templates its template imports -//! every slot, as ordinary overrides, until the network is reset. +//! every slot, as ordinary overrides, until the network is reset. An override with `keep_alive` +//! off stops keep-alive for the account it writes, until an override with it on writes that account, +//! which restarts every import that rewrites it. -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap, HashSet}; +use solana_account::Account; use solana_pubkey::Pubkey; +use surfpool_types::{KeepAliveAddress, KeepAliveImport}; -/// An account a scenario's override wrote through a template that imports keep-alive. +use super::TemplateRegistry; + +/// An account a scenario's override wrote. #[derive(Clone, Debug)] pub struct KeepAliveSource { pub template: String, @@ -15,6 +21,8 @@ pub struct KeepAliveSource { pub address: Pubkey, /// The override's values, which input and PDA addresses resolve against. pub values: HashMap, + /// The override's `keep_alive`: off, keep-alive stops for `address`. + pub keep_alive: bool, } /// Binds a template's keep-alive import to the account it rewrites every slot, resolved when an @@ -28,3 +36,400 @@ pub struct KeepAliveBinding { /// The address the import rewrites. pub address: Pubkey, } + +/// Binds a template's owned keep-alive import, which rewrites every local account of a kind, +/// resolved again each slot since the surfnet keeps loading accounts. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct KeepAliveKind { + /// The importing template's ID. + pub template: String, + /// The import's index in that template's `keep_alive` list. + pub import: usize, +} + +/// What keep-alive rewrites every slot. Kept in memory only, so a restart on a persistent database +/// stops keep-alive. +#[derive(Clone, Debug, Default)] +pub struct KeepAlive { + pub bindings: BTreeSet, + pub kinds: BTreeSet, + /// Accounts an override with `keep_alive` off wrote last. Keep-alive skips them but keeps + /// their bindings, so an override with it on restarts them all. + pub excluded: HashSet, +} + +impl KeepAlive { + /// Applies the overrides that wrote `touched`, in order: one with `keep_alive` off stops + /// keep-alive for its account, one with it on starts it again and adds its template's imports. + /// An import that rewrites a stopped account is bound too, and starts when the account does. + pub fn enroll(&mut self, registry: &TemplateRegistry, touched: Vec) { + for source in touched { + if !source.keep_alive { + if self.excluded.insert(source.address) { + info!("Keep-alive stops rewriting {}", source.address); + } + continue; + } + self.excluded.remove(&source.address); + let Some(template) = registry.get(&source.template) else { + continue; + }; + for (index, import) in template.keep_alive.iter().enumerate() { + if let KeepAliveAddress::Owned { owners, account } = &import.address { + let kind = KeepAliveKind { + template: template.id.clone(), + import: index, + }; + if self.kinds.insert(kind) { + info!( + "Keep-alive '{}' rewrites every local '{}' of {:?} every slot", + import.template, account, owners + ); + } + continue; + } + let Some(address) = import.address(source.address, &source.values) else { + warn!( + "Keep-alive '{}' of override template '{}' has no address for {}", + import.template, template.id, source.address + ); + continue; + }; + let binding = KeepAliveBinding { + template: template.id.clone(), + import: index, + address, + }; + if self.bindings.insert(binding) { + info!( + "Keep-alive '{}' rewrites {} every slot", + import.template, address + ); + } + } + } + } + + /// Each account to rewrite this slot with the import that rewrites it: every binding, then + /// every account `accounts_owned_by` returns for a kind's owners that holds the kind's account + /// type. Excluded accounts are skipped, and so is an account a keep-alive template already + /// rewrites. + pub fn targets<'r>( + &self, + registry: &'r TemplateRegistry, + accounts_owned_by: impl Fn(&Pubkey) -> Vec<(Pubkey, Account)>, + ) -> Vec<(&'r KeepAliveImport, Pubkey)> { + let import_of = |template: &str, index: usize| { + registry + .get(template) + .and_then(|template| template.keep_alive.get(index)) + }; + let mut seen = HashSet::new(); + let mut targets = Vec::new(); + for binding in &self.bindings { + if let Some(import) = import_of(&binding.template, binding.import) + && !self.excluded.contains(&binding.address) + && seen.insert((import.template.as_str(), binding.address)) + { + targets.push((import, binding.address)); + } + } + for kind in &self.kinds { + let Some(import) = import_of(&kind.template, kind.import) else { + continue; + }; + let KeepAliveAddress::Owned { owners, account } = &import.address else { + continue; + }; + let Some(discriminator) = registry + .get(&import.template) + .and_then(|template| template.account_discriminator(account)) + else { + continue; + }; + for owner in owners.iter().filter_map(|owner| owner.parse().ok()) { + for (address, owned) in accounts_owned_by(&owner) { + if owned.data.starts_with(discriminator) + && !self.excluded.contains(&address) + && seen.insert((import.template.as_str(), address)) + { + targets.push((import, address)); + } + } + } + } + targets + } +} + +#[cfg(test)] +pub(super) mod tests { + use super::*; + use crate::scenarios::registry::PYTH_V2_IDL_CONTENT; + + pub(crate) const OWNER: &str = "rec5EKMGg6MxZYaMdyBfgwp4d5rB9T1VQH5pJv5LtFJ"; + + pub(crate) const OWNED_OVERRIDES: &str = r#" +protocol: Example +version: v1 +account_type: State +raw_layout: true +templates: + - id: example-vault + name: Vault + description: Example vault + keep_alive: + - template: example-price-fresh + address: { type: owned, owner: rec5EKMGg6MxZYaMdyBfgwp4d5rB9T1VQH5pJv5LtFJ, account: PriceUpdateV2 } + values: { posted_slot: $slot } + - template: example-price-fresh + address: { type: input, value: oracle } + values: { posted_slot: $slot } + address: { type: pubkey } + properties: + - { path: balance, offset: 0, encoding: u64, description: Balance } + - { path: oracle, type: input, value_type: pubkey, description: The vault's oracle } +"#; + + const PRICE_KEEP_ALIVE: &str = r#" +protocol: Example +version: v1 +account_type: PriceUpdateV2 +templates: + - id: example-price-fresh + name: Fresh price + description: What the publisher republishes + address: { type: pubkey } + properties: + - { path: posted_slot, label: Posted slot, description: Posted slot } +"#; + + pub(crate) fn owned_registry(overrides: &str) -> TemplateRegistry { + let mut registry = TemplateRegistry::default(); + registry.load_raw_layout_overrides(overrides, "example"); + registry.load_keep_alive( + Some(serde_json::from_str(PYTH_V2_IDL_CONTENT).unwrap()), + PRICE_KEEP_ALIVE, + "example", + ); + registry + } + + fn account(owner: Pubkey, data: &[u8]) -> Account { + Account { + lamports: 1, + data: data.to_vec(), + owner, + executable: false, + rent_epoch: 0, + } + } + + fn source( + template: &str, + address: Pubkey, + oracle: Pubkey, + keep_alive: bool, + ) -> KeepAliveSource { + KeepAliveSource { + template: template.to_string(), + address, + values: HashMap::from([("oracle".to_string(), serde_json::json!(oracle.to_string()))]), + keep_alive, + } + } + + struct LocalAccounts { + registry: TemplateRegistry, + accounts: Vec<(Pubkey, Account)>, + price: [u8; 8], + } + + impl LocalAccounts { + fn new() -> Self { + let registry = owned_registry(OWNED_OVERRIDES); + let price = registry + .get("example-price-fresh") + .and_then(|template| template.account_discriminator("PriceUpdateV2")) + .unwrap() + .try_into() + .unwrap(); + Self { + registry, + accounts: Vec::new(), + price, + } + } + + fn load(&mut self, owner: Pubkey, discriminator: [u8; 8]) -> Pubkey { + let address = Pubkey::new_unique(); + let mut data = discriminator.to_vec(); + data.extend([0; 8]); + self.accounts.push((address, account(owner, &data))); + address + } + + fn targets(&self, keep_alive: &KeepAlive) -> Vec { + keep_alive + .targets(&self.registry, |owner| { + self.accounts + .iter() + .filter(|(_, account)| account.owner == *owner) + .cloned() + .collect() + }) + .into_iter() + .map(|(import, address)| { + assert_eq!(import.template, "example-price-fresh"); + address + }) + .collect() + } + } + + #[test] + fn an_owned_kind_rewrites_every_local_account_of_its_owner_and_type() { + let owner = OWNER.parse().unwrap(); + let mut surfnet = LocalAccounts::new(); + let oracle = surfnet.load(owner, surfnet.price); + let other_price = surfnet.load(owner, surfnet.price); + surfnet.load(owner, [9; 8]); + surfnet.load(Pubkey::new_unique(), surfnet.price); + + let mut keep_alive = KeepAlive::default(); + let vault = Pubkey::new_unique(); + keep_alive.enroll( + &surfnet.registry, + vec![source("example-vault", vault, oracle, true)], + ); + assert_eq!( + surfnet.targets(&keep_alive), + [oracle, other_price], + "the oracle is bound and owned, and is written once" + ); + + let loaded_later = surfnet.load(owner, surfnet.price); + assert_eq!( + surfnet.targets(&keep_alive), + [oracle, other_price, loaded_later] + ); + } + + #[test] + fn an_override_without_keep_alive_stops_it_for_its_account_until_one_with_it_plays() { + let owner = OWNER.parse().unwrap(); + let mut surfnet = LocalAccounts::new(); + let oracle = surfnet.load(owner, surfnet.price); + let other_price = surfnet.load(owner, surfnet.price); + let vault = Pubkey::new_unique(); + let mut keep_alive = KeepAlive::default(); + keep_alive.enroll( + &surfnet.registry, + vec![source("example-vault", vault, oracle, true)], + ); + + keep_alive.enroll( + &surfnet.registry, + vec![source("pyth-price-feed-v2", oracle, oracle, false)], + ); + assert_eq!(keep_alive.bindings.len(), 1, "its binding is kept"); + assert_eq!(surfnet.targets(&keep_alive), [other_price]); + + keep_alive.enroll( + &surfnet.registry, + vec![source("example-vault", vault, oracle, true)], + ); + assert_eq!( + surfnet.targets(&keep_alive), + [other_price], + "another account's override does not restart it" + ); + + keep_alive.enroll( + &surfnet.registry, + vec![source("pyth-price-feed-v2", oracle, oracle, true)], + ); + assert_eq!(surfnet.targets(&keep_alive), [oracle, other_price]); + } + + const INPUT_OVERRIDES: &str = r#" +protocol: Example +version: v1 +account_type: State +raw_layout: true +templates: + - id: example-vault + name: Vault + description: Example vault + keep_alive: + - template: example-price-fresh + address: { type: input, value: oracle } + values: { posted_slot: $slot } + address: { type: pubkey } + properties: + - { path: balance, offset: 0, encoding: u64, description: Balance } + - { path: oracle, type: input, value_type: pubkey, description: The vault's oracle } + - id: example-oracle + name: Oracle + description: Example oracle + address: { type: pubkey } + properties: + - { path: price, offset: 0, encoding: u64, description: Price } +"#; + + #[test] + fn keep_alive_on_again_restores_another_templates_binding() { + let surfnet = LocalAccounts { + registry: owned_registry(INPUT_OVERRIDES), + ..LocalAccounts::new() + }; + assert!( + surfnet + .registry + .get("example-oracle") + .unwrap() + .keep_alive + .is_empty(), + "re-enabling must not import anything itself" + ); + let oracle = Pubkey::new_unique(); + let mut keep_alive = KeepAlive::default(); + keep_alive.enroll( + &surfnet.registry, + vec![source("example-vault", Pubkey::new_unique(), oracle, true)], + ); + assert_eq!(surfnet.targets(&keep_alive), [oracle]); + + keep_alive.enroll( + &surfnet.registry, + vec![source("example-oracle", oracle, oracle, false)], + ); + assert!(surfnet.targets(&keep_alive).is_empty()); + + keep_alive.enroll( + &surfnet.registry, + vec![source("example-oracle", oracle, oracle, true)], + ); + assert_eq!(surfnet.targets(&keep_alive), [oracle]); + + let mut off_first = KeepAlive::default(); + off_first.enroll( + &surfnet.registry, + vec![ + source("example-oracle", oracle, oracle, false), + source("example-vault", Pubkey::new_unique(), oracle, true), + ], + ); + assert!(surfnet.targets(&off_first).is_empty()); + + off_first.enroll( + &surfnet.registry, + vec![source("example-oracle", oracle, oracle, true)], + ); + assert_eq!( + surfnet.targets(&off_first), + [oracle], + "a vault that played while the oracle was off still bound it" + ); + } +} diff --git a/crates/core/src/scenarios/registry.rs b/crates/core/src/scenarios/registry.rs index a80f6bb7f..890545d17 100644 --- a/crates/core/src/scenarios/registry.rs +++ b/crates/core/src/scenarios/registry.rs @@ -177,8 +177,9 @@ impl TemplateRegistry { } /// Fails on a keep-alive import the registry cannot apply: its template must be a loaded - /// keep-alive template that has every property it sets, and an input address must name a - /// pubkey input of the importing template. + /// keep-alive template that has every property it sets, an input address must name a pubkey + /// input of the importing template, and an owned address an account of the keep-alive + /// template's IDL. pub fn check_keep_alive_imports(&self) -> Result<(), String> { for template in self.templates.values() { for import in &template.keep_alive { @@ -208,6 +209,13 @@ impl TemplateRegistry { )); } } + if let KeepAliveAddress::Owned { account, .. } = &import.address + && keep_alive.account_discriminator(account).is_none() + { + return fail(format!( + "its IDL has no account '{account}' with a discriminator" + )); + } if let KeepAliveAddress::Input(name) = &import.address { let is_pubkey_input = template.properties.iter().any(|property| { &property.path == name @@ -1058,6 +1066,30 @@ templates: ); } + #[test] + fn an_owned_import_needs_its_account_in_the_keep_alive_idl() { + use crate::scenarios::keep_alive::tests::{OWNED_OVERRIDES, owned_registry}; + + owned_registry(OWNED_OVERRIDES) + .check_keep_alive_imports() + .unwrap(); + let error = + owned_registry(&OWNED_OVERRIDES.replace("account: PriceUpdateV2", "account: Nope")) + .check_keep_alive_imports() + .unwrap_err(); + assert!(error.contains("its IDL has no account 'Nope'"), "{error}"); + let error = keep_alive_registry(&KEEP_ALIVE_OVERRIDES.replace( + "address: { type: input, value: oracle }", + "address: { type: owned, owner: rec5EKMGg6MxZYaMdyBfgwp4d5rB9T1VQH5pJv5LtFJ, account: State }", + )) + .check_keep_alive_imports() + .unwrap_err(); + assert!( + error.contains("its IDL has no account 'State'"), + "a raw layout has no IDL: {error}" + ); + } + #[test] fn the_bundled_registry_resolves_every_keep_alive_import() { TemplateRegistry::new().check_keep_alive_imports().unwrap(); diff --git a/crates/core/src/surfnet/svm.rs b/crates/core/src/surfnet/svm.rs index 168e8e00d..9abdc80b2 100644 --- a/crates/core/src/surfnet/svm.rs +++ b/crates/core/src/surfnet/svm.rs @@ -1,6 +1,6 @@ use std::{ cmp::max, - collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque}, + collections::{BTreeMap, HashMap, HashSet, VecDeque}, ops::{ Bound::{Excluded, Unbounded}, RangeInclusive, @@ -105,7 +105,7 @@ use crate::{ rpc::utils::convert_transaction_metadata_from_canonical, scenarios::{ TemplateRegistry, account_data_values, - keep_alive::{KeepAliveBinding, KeepAliveSource}, + keep_alive::{KeepAlive, KeepAliveSource}, protocols::phoenix_eternal::v1::state_builder::{ prepare_phoenix_override, write_all_or_none, }, @@ -609,7 +609,7 @@ pub struct SurfnetSvm { pub scheduled_overrides: Box>>, /// The accounts keep-alive rewrites every slot, from the slot a scenario's override first /// touched them. Kept in memory only, so a restart on a persistent database stops keep-alive. - pub keep_alive_bindings: BTreeSet, + pub keep_alive: KeepAlive, /// Tracks accounts that should not be downloaded from the remote RPC. /// This includes accounts explicitly closed locally and accounts marked offline via cheatcodes. /// The key is the account pubkey as a string. If `include_owned_accounts` is true, @@ -893,7 +893,7 @@ impl SurfnetSvm { registered_idls: OverlayStorage::wrap(self.registered_idls.clone_box()), streamed_accounts: OverlayStorage::wrap(self.streamed_accounts.clone_box()), scheduled_overrides: OverlayStorage::wrap(self.scheduled_overrides.clone_box()), - keep_alive_bindings: self.keep_alive_bindings.clone(), + keep_alive: self.keep_alive.clone(), // Clone non-storage fields normally transactions_queued_for_confirmation: self.transactions_queued_for_confirmation.clone(), @@ -1403,7 +1403,7 @@ impl SurfnetSvm { streamed_accounts: streamed_accounts_db, recent_blockhashes: VecDeque::new(), scheduled_overrides: scheduled_overrides_db, - keep_alive_bindings: BTreeSet::new(), + keep_alive: KeepAlive::default(), offline_accounts: offline_accounts_db, genesis_slot: default_genesis_slot, genesis_updated_at: updated_at, @@ -2623,7 +2623,7 @@ impl SurfnetSvm { self.runbook_executions.clear(); self.streamed_accounts.clear()?; self.scheduled_overrides.clear()?; - self.keep_alive_bindings.clear(); + self.keep_alive = KeepAlive::default(); let current_time = chrono::Utc::now().timestamp_millis() as u64; self.updated_at = current_time; @@ -3404,7 +3404,7 @@ impl SurfnetSvm { /// 3. Optionally fetches fresh account data from remote if `fetch_before_use` is enabled /// 4. Applies the overrides to the account data /// 5. Updates the SVM state - /// 6. Applies keep-alive to every account in `keep_alive_bindings` + /// 6. Applies keep-alive to every account in `keep_alive` pub async fn materialize_overrides( &mut self, remote_ctx: &Option<(SurfnetRemoteClient, CommitmentConfig)>, @@ -3414,7 +3414,7 @@ impl SurfnetSvm { .await } - /// Applies keep-alive to every account in `keep_alive_bindings`, with the placeholders filled from + /// Applies keep-alive to every account in `keep_alive`, with the placeholders filled from /// `slot` and its Clock time. pub async fn apply_keep_alive( &mut self, @@ -3432,7 +3432,7 @@ impl SurfnetSvm { } /// Materializes scheduled overrides for a specific slot, adds the accounts they touch to - /// `keep_alive_bindings`, then applies keep-alive to every account on it. + /// `keep_alive`, then applies keep-alive to every account on it. pub async fn materialize_overrides_for_slot( &mut self, remote_ctx: &Option<(SurfnetRemoteClient, CommitmentConfig)>, @@ -3455,7 +3455,7 @@ impl SurfnetSvm { materialized } - /// Applies `overrides` in order and returns the scheduled ones whose template imports + /// Applies `overrides` in order and returns the scheduled ones that wrote their account, for /// keep-alive. `scheduled` overrides came out of this slot's schedule, which only registered /// scenarios fill: when one fails, the ones not reached yet go back into it. async fn apply_overrides( @@ -3528,19 +3528,14 @@ impl SurfnetSvm { } }; - // Joins keep-alive only once its write succeeds: a failed override leaves the account as - // if no scenario had touched it. - let mut keep_alive_source = None; - if scheduled - && let Some(template) = template_registry().get(&override_instance.template_id) - && !template.keep_alive.is_empty() - { - keep_alive_source = Some(KeepAliveSource { - template: template.id.clone(), - address: account_pubkey, - values: override_instance.values.clone(), - }); - } + // Joins or leaves keep-alive only once its write succeeds: a failed override leaves the + // account as if no scenario had touched it. + let mut keep_alive_source = scheduled.then(|| KeepAliveSource { + template: override_instance.template_id.clone(), + address: account_pubkey, + values: override_instance.values.clone(), + keep_alive: override_instance.keep_alive, + }); debug!( "Processing override {} for account {} (label: {:?})", @@ -3916,37 +3911,14 @@ impl SurfnetSvm { Ok(keep_alive_touched) } - /// Adds the accounts that `touched` makes keep-alive imports rewrite. + /// Starts or stops keep-alive for the accounts in `touched`. fn enroll_keep_alive(&mut self, touched: Vec) { - for source in touched { - let Some(template) = template_registry().get(&source.template) else { - continue; - }; - for (index, import) in template.keep_alive.iter().enumerate() { - let Some(address) = import.address(source.address, &source.values) else { - warn!( - "Keep-alive '{}' of override template '{}' has no address for {}", - import.template, template.id, source.address - ); - continue; - }; - let binding = KeepAliveBinding { - template: template.id.clone(), - import: index, - address, - }; - if self.keep_alive_bindings.insert(binding) { - info!( - "Keep-alive '{}' rewrites {} every slot", - import.template, address - ); - } - } - } + self.keep_alive.enroll(template_registry(), touched); } - /// The keep-alive overrides for every binding at `slot`. A missing account is fetched first when - /// `can_fetch` and not offline, otherwise skipped. + /// The keep-alive overrides for every account keep-alive rewrites at `slot`. A missing account + /// is fetched first when `can_fetch` and not offline, otherwise skipped. Owned kinds only reach + /// local accounts, so they never fetch. fn keep_alive_overrides(&self, slot: Slot, can_fetch: bool) -> Vec { // The time the open slot's Clock sysvar holds. The Clock advances by each slot's own // interval, so recomputing it from the slot would apply a changed slot interval to every @@ -3955,52 +3927,49 @@ impl SurfnetSvm { slot, unix_time_ms: self.updated_at, }; + let targets = self.keep_alive.targets(template_registry(), |owner| { + self.get_account_owned_by(owner).unwrap_or_else(|e| { + warn!("Keep-alive cannot read the accounts of {}: {}", owner, e); + Vec::new() + }) + }); let mut instances = Vec::new(); - for binding in &self.keep_alive_bindings { - let Some(import) = template_registry() - .get(&binding.template) - .and_then(|template| template.keep_alive.get(binding.import)) - else { - continue; - }; - let missing = match self.get_account(&binding.address) { + for (import, address) in targets { + let missing = match self.get_account(&address) { Ok(Some(_)) => false, Ok(None) if can_fetch && !self .offline_accounts - .contains_key(&binding.address.to_string()) + .contains_key(&address.to_string()) .unwrap_or(false) => { true } Ok(None) => { - debug!("Keep-alive skips {}: not in this surfnet", binding.address); + debug!("Keep-alive skips {}: not in this surfnet", address); continue; } Err(e) => { - warn!("Keep-alive cannot read {}: {}", binding.address, e); + warn!("Keep-alive cannot read {}: {}", address, e); continue; } }; let values = match import.values_at(&clock) { Ok(values) => values, Err(e) => { - warn!( - "Keep-alive '{}' skips {}: {}", - import.template, binding.address, e - ); + warn!("Keep-alive '{}' skips {}: {}", import.template, address, e); continue; } }; let mut instance = OverrideInstance::new( import.template.clone(), 0, - surfpool_types::AccountAddress::Pubkey(binding.address.to_string()), + surfpool_types::AccountAddress::Pubkey(address.to_string()), ) .with_values(values) .with_label(format!("keep-alive {}", import.template)); - instance.id = format!("keep-alive:{}:{}", import.template, binding.address); + instance.id = format!("keep-alive:{}:{}", import.template, address); instance.fetch_before_use = missing; instances.push(instance); } @@ -9734,4 +9703,28 @@ mod tests { .expect("the override must notify the account's subscribers"); assert_eq!(update.data.decode(), Some(account.data)); } + + #[tokio::test] + async fn a_written_override_without_keep_alive_excludes_its_account_until_one_with_it() { + const SLOT: u64 = 500; + let (mut svm, _simnet_events_rx, _geyser_events_rx) = SurfnetSvm::default(); + let (account_pubkey, instance) = scheduled_override_fixture(&mut svm); + let mut play = async |keep_alive: bool, values: HashMap| { + let mut instance = instance.clone().with_values(values); + instance.keep_alive = keep_alive; + svm.scheduled_overrides.store(SLOT, vec![instance]).unwrap(); + svm.materialize_overrides_for_slot(&None, SLOT) + .await + .unwrap(); + svm.keep_alive.excluded.contains(&account_pubkey) + }; + + let failed = HashMap::from([("nope".to_string(), serde_json::json!(1))]); + assert!( + !play(false, failed).await, + "a failed write excludes nothing" + ); + assert!(play(false, instance.values.clone()).await); + assert!(!play(true, instance.values.clone()).await); + } } diff --git a/crates/sdk-node/surfpool-sdk/kit/generated/OverrideInstance.ts b/crates/sdk-node/surfpool-sdk/kit/generated/OverrideInstance.ts index 0bf5926b1..c52415578 100644 --- a/crates/sdk-node/surfpool-sdk/kit/generated/OverrideInstance.ts +++ b/crates/sdk-node/surfpool-sdk/kit/generated/OverrideInstance.ts @@ -36,6 +36,11 @@ enabled: boolean, * Whether to fetch fresh account data just before transaction execution */ fetchBeforeUse?: boolean, +/** + * Whether keep-alive rewrites this account from this slot on. Off, keep-alive stops for the + * account, so its fields age as the clock moves on, until a later override turns it on again. + */ +keepAlive?: boolean, /** * Account address to override - use pubkey for known addresses or pda for derived addresses */ diff --git a/crates/types/src/rpc_endpoints.json b/crates/types/src/rpc_endpoints.json index bc5b02991..6517dd7f0 100644 --- a/crates/types/src/rpc_endpoints.json +++ b/crates/types/src/rpc_endpoints.json @@ -861,6 +861,7 @@ "label": "Option (An optional label for this override instance)", "enabled": "bool (Indicates whether this override instance is enabled)", "fetchBeforeUse": "bool (Indicates whether to fetch the latest on-chain account data before applying overrides)", + "keepAlive": "bool (Optional, defaults to true. If false, keep-alive stops rewriting this account from this slot on, and its fields age as the clock moves on. A later override with true starts it again)", "account": "AccountAddress (The account this override targets, as {\"pubkey\": \"\"} or {\"pda\": {\"programId\": \"\", \"seeds\": [ ... ]}})" } } diff --git a/crates/types/src/scenarios.rs b/crates/types/src/scenarios.rs index 46a0c1088..71aab50b7 100644 --- a/crates/types/src/scenarios.rs +++ b/crates/types/src/scenarios.rs @@ -592,6 +592,14 @@ pub struct OverrideInstance { #[serde(default)] #[cfg_attr(feature = "ts-bindings", ts(as = "Option", optional))] pub fetch_before_use: bool, + /// Whether keep-alive rewrites this account from this slot on. Off, keep-alive stops for the + /// account, so its fields age as the clock moves on, until a later override turns it on again. + #[schemars( + description = "If false, keep-alive stops rewriting this account from this slot on, so its fields age. A later override with true starts it again. Defaults to true" + )] + #[serde(default = "default_keep_alive")] + #[cfg_attr(feature = "ts-bindings", ts(as = "Option", optional))] + pub keep_alive: bool, /// Account address to override - use pubkey for known addresses or pda for derived addresses #[schemars( description = "Account address: either {\"pubkey\": \"base58_address\"} or {\"pda\": {\"programId\": \"...\", \"seeds\": [...]}}" @@ -599,6 +607,10 @@ pub struct OverrideInstance { pub account: AccountAddress, } +fn default_keep_alive() -> bool { + true +} + impl OverrideInstance { pub fn new(template_id: String, scenario_relative_slot: Slot, account: AccountAddress) -> Self { Self { @@ -609,6 +621,7 @@ impl OverrideInstance { label: None, enabled: true, fetch_before_use: false, + keep_alive: true, account, } } @@ -1563,6 +1576,20 @@ pub enum YamlKeepAliveAddress { program_id: String, seeds: Vec, }, + /// Every local account that `owner` owns and whose data starts with the discriminator of + /// `account` in the keep-alive template's IDL. + Owned { + owner: YamlOwners, + account: String, + }, +} + +/// One program ID or a list of them. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum YamlOwners { + One(String), + Many(Vec), } impl From for KeepAliveImport { @@ -1579,6 +1606,13 @@ impl From for KeepAliveImport { seeds: seeds.into_iter().map(Into::into).collect(), }) } + YamlKeepAliveAddress::Owned { owner, account } => KeepAliveAddress::Owned { + owners: match owner { + YamlOwners::One(owner) => vec![owner], + YamlOwners::Many(owners) => owners, + }, + account, + }, }; KeepAliveImport { template: yaml.template, @@ -1613,10 +1647,18 @@ pub enum KeepAliveAddress { Input(String), /// A fixed or derived address. PDA seeds resolve against the importing override's values. Account(AccountAddress), + /// Every account in the surfnet that one of `owners` owns and whose data starts with the + /// discriminator of `account` in the keep-alive template's IDL, including accounts loaded after + /// the override played. Only local accounts are written, never fetched. + Owned { + owners: Vec, + account: String, + }, } impl KeepAliveImport { - /// The address to write, given the address the importing override wrote and its values. + /// The address to write, given the address the importing override wrote and its values. `None` + /// for an owned import, which names no single account. pub fn address( &self, written: Pubkey, @@ -1629,6 +1671,7 @@ impl KeepAliveImport { .as_str() .and_then(|address| Pubkey::from_str(address).ok()), KeepAliveAddress::Account(address) => address.resolve(Some(values)), + KeepAliveAddress::Owned { .. } => None, } } @@ -1663,6 +1706,16 @@ impl KeepAliveImport { format!("keep-alive '{template}': invalid address '{address}': {e}") })?; } + if let KeepAliveAddress::Owned { owners, .. } = &self.address { + if owners.is_empty() { + return Err(format!("keep-alive '{template}' names no owner")); + } + for owner in owners { + Pubkey::from_str(owner).map_err(|e| { + format!("keep-alive '{template}': invalid owner '{owner}': {e}") + })?; + } + } let clock = KeepAliveClock { slot: 0, unix_time_ms: 0, @@ -1674,6 +1727,17 @@ impl KeepAliveImport { } impl OverrideTemplate { + /// The discriminator of `account` in this template's IDL, when it has one. + pub fn account_discriminator(&self, account: &str) -> Option<&[u8]> { + self.idl + .as_ref()? + .accounts + .iter() + .find(|idl_account| idl_account.name == account) + .map(|idl_account| idl_account.discriminator.as_slice()) + .filter(|discriminator| !discriminator.is_empty()) + } + /// The input properties keep-alive imports read addresses from. They are never written. pub fn keep_alive_inputs(&self) -> impl Iterator { self.keep_alive @@ -2000,6 +2064,27 @@ mod tests { assert_eq!(values["fixed"], json!(3)); } + #[test] + fn an_override_keeps_its_account_alive_unless_it_opts_out() { + let instance = |extra: serde_json::Value| { + let mut value = json!({ + "id": "o", + "templateId": "t", + "values": {}, + "scenarioRelativeSlot": 0, + "enabled": true, + "account": { "pubkey": "11111111111111111111111111111111" }, + }); + value + .as_object_mut() + .unwrap() + .extend(extra.as_object().unwrap().clone()); + serde_json::from_value::(value).unwrap() + }; + assert!(instance(json!({})).keep_alive); + assert!(!instance(json!({ "keepAlive": false })).keep_alive); + } + #[test] fn a_file_level_keep_alive_is_inherited_unless_a_template_sets_its_own() { let file: super::YamlOverrideTemplateCollection = serde_yaml::from_str( @@ -2067,6 +2152,40 @@ templates: "template: t\naddress: { type: pda, program_id: nope, seeds: [] }\nvalues: { slot: 1 }\n", "invalid address", ); + refused( + "template: t\naddress: { type: owned, owner: nope, account: A }\nvalues: { slot: 1 }\n", + "invalid owner 'nope'", + ); + refused( + "template: t\naddress: { type: owned, owner: [], account: A }\nvalues: { slot: 1 }\n", + "names no owner", + ); + } + + #[test] + fn an_owned_import_takes_one_owner_or_a_list() { + use solana_pubkey::Pubkey; + let (first, second) = (Pubkey::new_unique(), Pubkey::new_unique()); + let owned = |owner: String| { + keep_alive(&format!( + "template: t\naddress: {{ type: owned, owner: {owner}, account: Price }}\nvalues: {{ slot: $slot }}\n" + )) + }; + let expected = |owners: &[Pubkey]| super::KeepAliveAddress::Owned { + owners: owners.iter().map(Pubkey::to_string).collect(), + account: "Price".to_string(), + }; + let one = owned(format!("\"{first}\"")); + assert_eq!(one.address, expected(&[first])); + one.validate().unwrap(); + let many = owned(format!("[\"{first}\", \"{second}\"]")); + assert_eq!(many.address, expected(&[first, second])); + many.validate().unwrap(); + assert_eq!( + many.address(first, &HashMap::new()), + None, + "it names no single account" + ); } #[test]