Skip to content

Chore: cleanup repo for oss publish.#18

Merged
braden-thompson-cb merged 6 commits into
mainfrom
sanitize
May 19, 2026
Merged

Chore: cleanup repo for oss publish.#18
braden-thompson-cb merged 6 commits into
mainfrom
sanitize

Conversation

@braden-thompson-cb

@braden-thompson-cb braden-thompson-cb commented May 14, 2026

Copy link
Copy Markdown
Contributor

Description

  1. Deleted scripts since these were not security audited and were only used for dev testing
  2. Deleted solana runbooks that reference the scripts
  3. Renamed solana program to stable swapper to match ethereum
  4. Added solana CI

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Dependency update
  • Documentation
  • Refactor / cleanup
  • Other (describe below)

Checklist

  • Tests are included and passing
  • Code is formatted and linted
  • Build succeeds
  • Documentation is updated for any public API changes
  • All commits are signed

@cb-heimdall

cb-heimdall commented May 14, 2026

Copy link
Copy Markdown

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@denys-cb

Copy link
Copy Markdown

Code Review — PR #18 (Chore: cleanup repo for OSS publish)

Overview

Three concerns rolled into one cleanup PR ahead of OSS publish:

  1. Removes EVM deployment + verification scripts (DeployStableSwapper.s.sol, VerifyDeployment.s.sol, GenerateStorageLocation.s.sol) and their unit tests, with READMEs updated to point at out-of-repo tooling.
  2. Removes Solana ops scripts (scripts/*.ts), runbooks, and migrations/deploy.ts.
  3. Renames the Solana program from scaas-liquiditystable-swapper (crate, lib, declare_id! module, Anchor.toml, Cargo.lock, README, tests).
  4. Splits the CI workflow into two jobs (evm, solana) and adds a Solana/Anchor build+test job.
  5. Renames the appStable test fixture token to customStable across the EVM test suite.

Net +482 / -7618 lines — mostly deletions.

Code quality

  • Renames are consistent across all surfaces I checked: Anchor.toml (localnet/devnet/mainnet), Cargo.toml (name/lib.name), programs/stable-swapper/src/lib.rs (module), tests/stable-swapper.ts (workspace lookup anchor.workspace.stableSwapper and target/types/stable_swapper), and Cargo.lock.
  • Test fixture rename appStable → customStable is mechanical and correctly applied across all evm/test/unit/StableSwapper/*.t.sol, including the renamed fuzz test names (testFuzz_swap_transfersTokensCorrectly_usdcToCustomStable, etc.). No stale references remain.
  • evm/foundry.toml strips a trailing space — harmless.
  • Existing test coverage continues to cover the contract surface; the only test removal is DeployStableSwapper.t.sol, which is appropriate since the deployment script itself is gone.

Issues / suggestions

1. Dangling subheading in top-level README (minor, doc bug)
After the rewrite, this section is structurally broken:

Shared across both implementations:
- ...

Implementation-specific features (see each README for details):

## Implementations

The "Implementation-specific features" line is a header with no content beneath it before the next H2 — readers will see a label that promises a list and then immediately jumps to the implementations table. Either inline a one-line pointer ("See each implementation's README below.") or drop the line entirely.

2. Solana CI: cargo install --git ... anchor-cli will be very slow on every run (perf)
cargo install --git https://github.com/coral-xyz/anchor --tag v${ANCHOR_VERSION} anchor-cli --locked compiles Anchor from source on every PR with no caching, which typically adds 5–10 minutes per run. Consider either:

  • Caching ~/.cargo and ~/.rustup via Swatinem/rust-cache@… (also benefits anchor build), or
  • Installing Anchor via avm from a prebuilt binary, or
  • Pinning to a commit SHA (--rev) instead of a tag for stronger reproducibility.

3. Solana CI: no fmt/clippy/test gating beyond anchor test (gap)
The EVM job runs forge fmt --check and forge build --sizes before tests. The Solana job has no equivalent (cargo fmt -- --check, cargo clippy -- -D warnings). Worth adding for OSS hygiene.

4. Solana CI: solana_release URL pinned only by tag (supply chain, low risk)
https://github.com/anza-xyz/agave/releases/download/v${SOLANA_VERSION}/... relies on the tag being immutable. Anza tags are stable in practice, but if you want hermetic CI consider checksum verification (sha256sum -c) of the downloaded tarball. The harden-runner step runs in audit mode, not block, so it won't enforce egress policy here.

5. Anchor.toml localnet and mainnet share program ID pqgqKahpG1y2wsgxFhzaAnkV1cL9vk8MSg9qm4q646F (pre-existing, flag for follow-up)
Not introduced by this PR, but now that this is going OSS it's worth confirming whether mainnet and localnet truly share the same address or this is a leftover placeholder. The CI rewrites both to a fresh ephemeral key for tests, so this doesn't affect CI — only documentation/operational accuracy.

6. TokenVault.reserved_amount deprecation note loses ticket reference (nit)
The state struct's deprecation note changed from "...removed in STBLE-2811" to "...removed." That's correct for OSS audiences (don't leak internal tracker IDs), but consider replacing with a one-line rationale ("reserved_amount semantics replaced by direct vault balance accounting") so future readers understand why the field is retained instead of dropped.

Risks

  • Behavior: Zero on-chain or contract logic changes. EVM StableSwapper.sol and Solana lib.rs semantics are untouched. Risk is purely tooling/docs.
  • Reproducibility: Anyone relying on the deleted EVM deployment scripts or Solana ops scripts must use out-of-repo tooling. Acceptable given the OSS-publish goal; worth a callout in release notes.
  • Solana program rename: The crate name change does not change declare_id! values or on-chain program IDs, so existing deployments remain reachable. The rename only affects local builds and IDL artifact names.
  • CI: New Solana job adds ~5–10 min to PR CI until Anchor install is cached.

Verdict

LGTM with the README dangling-subheading fix and the CI caching note worth addressing before merge. Other items are non-blocking polish.

Comment thread README.md Outdated
- **Pause Controls** -- Independent toggles for swap and liquidity operations
- **Slippage Protection** -- Users specify a minimum output amount per swap

Implementation-specific features (see each README for details):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one can be removed?

Comment thread solana/Anchor.toml

- name: Build Anchor program
working-directory: solana
run: anchor build

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any formatting needed before the build like in evm?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

denys-cb
denys-cb previously approved these changes May 15, 2026
@braden-thompson-cb braden-thompson-cb merged commit 5759871 into main May 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants