ci: add testnet smoke test workflow and documentation - #71
ci: add testnet smoke test workflow and documentation#71SudiptaPaul-31 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Good news first: the workflow trigger is safe — workflow_dispatch only, no pull_request/pull_request_target, so this can't be abused by a fork PR to exfiltrate secrets, and no ${{ secrets.* }} are referenced.
Blocker: the smoke test doesn't appear to actually run successfully
ADMIN_SECRET/DEPLOYER_SECRETare placeholder strings that aren't valid 56-char Stellar StrKey seeds (they're 60 chars) — the--deploypath will fail immediately indeploy.sh.- Deployer/admin accounts are never Friendbot-funded (only business/creator1/creator2 are).
- The default (non-
--deploy) path expects.env.testnet, but that file is gitignored and never produced or cached by the workflow — a fresh checkout hits ".env.testnet not found" every time. NATIVE_XLM_ISSUER(aG...account address) is passed asPayoutAsset.token, but that field expects aC...SAC contract address —fund_campaign's token transfer will fail against a non-contract address.assert_success $?after eachinvoke_contractcall is dead code underset -euo pipefail(the script already aborts on failure before reaching it), and the final "escrow balance should be 0" check is just printed rather than parsed/asserted — this doesn't verify on-chain state the way the PR description claims.
Minor
actions/upload-artifact@v3is deprecated by GitHub; consider bumping to v4.
Since this closes #47 as a pre-release verification tool, it'd be good to confirm it actually completes a full run (locally against a real testnet or via the manual workflow dispatch) before merging — as written it looks like it will fail before completing the lifecycle it's meant to verify.
JamesVictor-O
left a comment
There was a problem hiding this comment.
Good addition in principle — a real-network smoke test against actual stellar contract invoke calls is genuinely useful pre-release coverage that the mocked unit tests can't provide, and the script's structure (assert helpers, cleanup trap, --deploy/--keep-env flags) is solid. The regular CI checks (Format/Clippy/Test/Build) pass because this workflow is workflow_dispatch-only and isn't exercised by them — but I did trace through what happens if it's actually triggered, and found a blocking issue there.
Blocking: the workflow's own generated .env uses non-functional, hardcoded fake credentials, in the "Generate test environment" step:
DEPLOYER_SECRET=SBKXS3ZCQNM6PJZ3Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7
ADMIN_ADDRESS=GBUQWP3BOUZX34ULNQG23RQ6F4BFSRXVHAEVZQOTPY5A37YPUYSFVROA
ADMIN_SECRET=SBKXS3ZCQNM6PJZ3Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7Q7A few problems with this:
- That's not a valid Stellar
StrKey(it fails the checksum — the repeatingQ7Q7...pattern is a placeholder, not a real generated key), so anystellarCLI call that tries to sign withADMIN_SECRET/DEPLOYER_SECRETwill fail outright. - Even with a syntactically valid key, it's never funded via Friendbot (unlike
BUSINESS_SECRET/CREATOR1_SECRET/CREATOR2_SECRET, which the script correctly generates fresh and funds each run) — so it wouldn't have a sequence number to sign with. FEE_BPSis required bydeploy.sh(: "${FEE_BPS:?Set FEE_BPS in .env...}") but isn't set anywhere in this generated.env, so the--deploypath fails immediately.- More generally, hardcoding any secret-shaped value directly in the workflow YAML is the wrong pattern even for a "test-only" account — it should either be generated fresh at runtime the same way the business/creator keys are, or sourced from a GitHub Actions secret (
${{ secrets.ADMIN_SECRET }}etc.) if a stable admin identity across runs is actually needed.
Net effect: as written, triggering this workflow (with or without --deploy) will fail the first time it touches ADMIN_SECRET/DEPLOYER_SECRET — e.g. the get_campaign verification calls in the smoke test script, or deploy.sh itself. Worth actually running this against testnet once fixed, since the PR description notes it was only "verified syntactically" so far.
Everything else (README/ARCHITECTURE docs, the lifecycle-driving logic in the script) looks good — this is a fairly contained fix once the admin/deployer credential handling is sorted out.
There was a problem hiding this comment.
found several issues that would prevent this smoke test from actually working (no CI is configured yet to catch these):
scripts/testnet-smoke-test.sh:165— the campaign's payout asset is set to a raw Stellar account (G...) address, butPayoutAsset.tokenmust be a deployed SEP-41 token contract (C...) address.fund_campaign'stoken::Client::new(...).transfer(...)will fail against a non-contract account. For native XLM usestellar contract id asset --asset native --network testnetto get the SAC address..github/workflows/testnet-smoke-test.yml:60—stellar keys generateis called with no NAME argument and its output is grepped for a literal"Secret:"line, but the CLI requires a positional NAME and doesn't print the secret on success — it must be fetched separately viastellar keys secret NAME. As written,DEPLOYER_SECRET/ADMIN_SECRET(and the analogous vars in the script) end up empty..github/workflows/testnet-smoke-test.yml:109— usesactions/upload-artifact@v3, which GitHub retired on 2025-01-30; the upload step will fail on every run..github/workflows/testnet-smoke-test.yml:16—deploy_freshdefaults tofalse, but GitHub-hosted runners start from a clean checkout each time and.env.testnetisn't committed, so the default invocation always fails with ".env.testnet not found".scripts/testnet-smoke-test.sh:79—assert_success $? "..."is called after eachinvoke_contract, but underset -euo pipefaila nonzero exit already aborts the script beforeassert_successruns, so its diagnostic message is dead code.scripts/testnet-smoke-test.sh:229—grep -oPuses GNU-only PCRE mode, which macOS's default BSDgrepdoesn't support, breaking local runs on the platform the new README section documents this script for.
Recommend fixing the asset address and key-generation issues at minimum before merging, since those block the smoke test from ever completing a real run.
|
Merged manually after fixing 6 bugs that would have prevented the smoke test from ever running: (1) stellar keys generate now uses named aliases; (2) native XLM address derived via stellar contract id asset --asset native instead of hardcoding a G-address; (3) grep -oP replaced with grep -oE for macOS compatibility; (4) assert_success calls after invoke_contract were dead code under set -e, replaced with inline error handling; (5) upload-artifact@v3 (retired Jan 2025) upgraded to @v4; (6) deploy_fresh default changed to true since CI runners have no .env.testnet. Thank you @SudiptaPaul-31! |
Summary
Add a comprehensive end-to-end smoke test for verifying the full campaign lifecycle against a real testnet deployment. This closes #47 by providing a pre-release verification path that exercises the actual deployed contracts through the
stellar contract invokeCLI, not just unit tests in a mock environment.What's included
New files
scripts/testnet-smoke-test.sh— Bash script that orchestrates the complete campaign lifecycle:create_campaign→fund_campaign→apply_to_campaign(×2) →approve_creator(×2) →submit_proof(×2) →approve_submission(×2) →claim_payment(×2)--deployto deploy fresh contracts and--keep-envto preserve test artifacts.github/workflows/testnet-smoke-test.yml— Manually-triggered GitHub Actions workflow:Documentation updates
README.md— Added "End-to-end testnet smoke test" section with:./scripts/testnet-smoke-test.sh,--deploy,--keep-env)docs/ARCHITECTURE.md— Added "Testing strategy" section explaining:Why this matters
stellarCLI that users depend onTesting
cargo test --workspacestill passesset -euo pipefail) and cleanup via trapAcceptance criteria (from #47)
Related
Closes #47