Load Surfpool test fixtures with --skip-deploy and allow extra launch arguments by xternet · Pull Request #5221 · otter-sec/anchor

xternet · 2026-10-23T21:15:56Z

When using Surfpool, --skip-deploy should skip program deployment, not configured test fixtures.

Fixtures: load and verify account/genesis fixtures; reject malformed input and bound zstd decompression.
Arguments: add [surfpool].extra_args; reject Anchor-managed flags.
Tests/docs: group tests by behavior; update both references and the example.
Compatibility: exhaustive SurfpoolConfig literals need extra_args; normal deployment is unchanged.

vercel · 2026-10-23T21:16:03Z

@xternet is attempting to deploy a commit to the OtterSec Team on Vercel.

A member of the Team first needs to authorize it.

swaroop-osec · 2026-10-28T05:13:59Z

@@ -1,5 +1,7 @@
 # Changelog

+- cli: Honor account-file, directory and genesis fixtures on the Surfpool test backend.


This should be added under Unreleased

swaroop-osec · 2026-10-28T05:16:49Z

+
+```toml
+[surfpool]
+extra_args = ["--no-studio"]


Surfpool rejects any flag if given twice. This example breaks anchor test since Anchor already passes --no-studio on every run.

swaroop-osec · 2026-10-28T06:12:33Z

        flags.push("--no-studio".to_string());
    }

+    if let Some(extra_args) = surfpool_config.as_ref().and_then(|c| c.extra_args.as_ref()) {


Setting extra_args = ["--no-deploy"] without --skip-deploy passes both --no-deploy and --legacy-anchor-compatibility.

Reject --no-deploy, --legacy-anchor-compatibility, and --anchor-test-config-path in extra_args. Update the documentation to clarify that these flags are not allowed because Anchor sets them through --skip-deploy.

swaroop-osec · 2026-10-28T06:19:32Z

 block_production_mode = "clock"
 ```

+### `extra_args`


Please update the documentation so that both documentation (v1 & v2) matches.

swaroop-osec · 2026-10-28T06:21:39Z

 url = "https://anchor.projectserum.com"
 ```
+
+### Account and genesis fixtures on Surfpool


We should move this section up, before ## toolchain.

swaroop-osec · 2026-10-28T06:33:16Z

+            let text = parts[0]
+                .as_str()
+                .ok_or_else(|| anyhow!("invalid account data"))?;
+            match parts[1].as_str() {


Can we also add support for base64+zstd

swaroop-osec · 2026-10-28T06:35:12Z

+        for (address, image) in images {
+            let key: Pubkey = address.parse()?;
+            let actual = client
+                .get_account_with_commitment(&key, CommitmentConfig::processed())?


Can we use get_multiple_accounts_with_commitment here instead of get_account_with_commitment

swaroop-osec · 2026-10-28T06:41:10Z

+            let data_state = UpgradeableLoaderState::ProgramData {
+                slot: 0,
+                upgrade_authority_address: Some(if program.upgradeable == Some(true) {
+                    cfg.wallet_kp()?.pubkey()


We are reading cfg.wallet_kp()?.pubkey() once per genesis entry. Read it once before the loop and reuse it.

swaroop-osec · 2026-10-28T06:43:25Z

+    if flags.first().map(String::as_str) != Some("--snapshot") {
+        return Ok(());
+    }
+    {


nit: The bare { … } block here doesn’t seem to do anything here. Could you remove the braces and format the code.

swaroop-osec · 2026-10-28T06:43:33Z

+        if !elf.starts_with(b"\x7fELF") {
+            bail!("Genesis program {} is not an ELF file", path.display());
+        }
+        {


nit: The bare { … } block here doesn’t seem to do anything here. Could you remove the braces and format the code.

swaroop-osec · 2026-10-28T06:47:48Z

Another thing I noticed is the same fixture files are parsed three different ways.

account_dir_pubkeys -> JSON file with no pubkey is silently skipped (continue).
validator_supplied_account_pubkeys -> Has a new "-" handling
load -> File with no pubkey is an error

It would be better to extract the logic into a helper function and reuse it across all three places.

swaroop-osec · 2026-10-28T06:57:27Z

Please don't use LLMs to write PR descriptions or documentation without reviewing the generated content first. Refer to the PR guidelines for more details.

Share fixture parsing, bound compressed data decoding, and batch account verification. Reject managed launch arguments, reuse the genesis authority, and correct the fixture documentation and examples.

xternet · 2026-10-28T11:21:19Z

Thanks for the review. Fixed in cfa76dd:

✅ Shared fixture parsing and base64+zstd support.
✅ Batched reads and one-time wallet-key reading.
✅ Rejected conflicting deployment flags.
✅ Fixed both docs, example and changelog.
✅ Removed redundant blocks.

Local checks pass: 171 tests in each Solana configuration, 4 integration cases, build, formatting and Clippy.

Also carried into #5222 and tested.

swaroop-osec · 2026-10-28T13:30:02Z

@xternet docs-v2 formatting was changed completely.

xternet · 2026-10-28T16:26:49Z

You’re right, the rewrite was unnecessary. Restored docs-v2’s formatting in ca72d49 and carried it into #5222. Local checks pass.

jamie-osec · 2026-10-29T13:07:15Z

Can we isolate all tests into a tests/ directory or something? It seems like this PR is mostly tests and it makes it hard to judge complexity currently.
Also, I see one file is named review_tests.rs - if these are tests for issues raised in this PR review, this naming is nonsensical for commited code. Please reorganise into a coherent structure

xternet · 2026-10-29T20:39:56Z

Done. Tests are now grouped by behavior in tests/ modules, with review-specific file and test names removed.

akash-osec · 2026-10-29T07:18:30Z

+
+fn image(owner: String, data: &[u8], executable: bool) -> Value {
+    json!({"owner":owner,"data":STANDARD.encode(data),"lamports":rent_exempt_minimum(data.len() as u64),
+        "executable":executable,"rentEpoch":RENT_EPOCH_NEVER,"parsedData":null})


here assigns RENT_EPOCH_NEVER to both program and program-data accounts; whereas verify does not compare rentEpoch, so the mismatch passes verification.

akash-osec · 2026-10-29T07:24:56Z

    }

+    if let Some(extra_args) = surfpool_config.as_ref().and_then(|c| c.extra_args.as_ref()) {
+        flags.extend(extra_args.iter().cloned());


setting extra_args = ["--no-studio"] cause duplicate args in extra_args; but it shouldnt allow duplicate runtime flags --no-studio or --port right? causing surfpool startup failures

xternet added 2 commits October 23, 2026 23:15

Allow additional Surfpool test launch arguments

4f4b38d

Load declared Surfpool fixtures when deployment is skipped

c8752b5

xternet requested review from chen-robert, jamie-osec and renato-osec as code owners October 23, 2026 21:15

xternet mentioned this pull request Oct 23, 2026

Complete isolated historical test suites and templates #5222

Open

swaroop-osec requested changes Oct 28, 2026

View reviewed changes

fix(cli): address Surfpool fixture review

cfa76dd

Share fixture parsing, bound compressed data decoding, and batch account verification. Reject managed launch arguments, reuse the genesis authority, and correct the fixture documentation and examples.

fix(cli): preserve documentation style in Surfpool fixture follow-up

ca72d49

test(cli): organize Surfpool fixture tests

4432bcb

akash-osec requested changes Oct 30, 2026

View reviewed changes

fix(cli): verify fixture epochs and reject duplicate launch flags

2749800

		@@ -1,5 +1,7 @@
		# Changelog

		- cli: Honor account-file, directory and genesis fixtures on the Surfpool test backend.

Conversation

xternet commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Oct 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swaroop-osec commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swaroop-osec commented Oct 28, 2026

Uh oh!

xternet commented Oct 28, 2026

Uh oh!

swaroop-osec commented Oct 28, 2026

Uh oh!

xternet commented Oct 28, 2026

Uh oh!

jamie-osec commented Oct 29, 2026

Uh oh!

xternet commented Oct 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xternet commented Oct 23, 2026 •

edited

Loading

swaroop-osec commented Oct 28, 2026 •

edited

Loading