Skip to content

fix(tholos): pin admin in __constructor to close initialize front-running - #214

Merged
collinsezedike merged 3 commits into
mainfrom
fix/tholos-v1-initialize-front-running
Sep 11, 2026
Merged

fix(tholos): pin admin in __constructor to close initialize front-running#214
collinsezedike merged 3 commits into
mainfrom
fix/tholos-v1-initialize-front-running

Conversation

@collinsezedike

Copy link
Copy Markdown
Collaborator

Summary

initialize (contracts/tholos/src/lib.rs) used to take admin as a caller-supplied parameter and only check that address's signature, so whoever's initialize call landed first, not necessarily the party who paid to deploy, became the permanent admin. Soroban deployment and initialization are separate transactions, so a third party watching the mempool could front-run a legitimate deployer's initialize call with their own.

admin is now pinned by a __constructor function, which Soroban invokes atomically as part of the same operation that creates the contract instance. initialize no longer accepts an admin parameter at all; it authenticates against whatever __constructor already fixed. This mirrors the already-merged fix for the same issue in contracts/tholos-v2 (#154, PR #183), applying the same pattern to v1 for consistency, as the original issue requested.

Closes #158

Also updated for consistency with the new deploy shape:

  • docs/src/CONTRACT.md: new __constructor entry, initialize's signature and CLI example updated, NotInitialized table row corrected (admin-gated calls like update_resolvers no longer return it, since admin is fixed at deploy, not at initialize)
  • docs/src/CONTRACT_V2.md: removed a now-stale "unlike v1" comparison in initialize's doc, since v1 uses the same __constructor pattern now
  • docs/src/DEPLOYMENT.md: deploy example updated to pass admin as a constructor argument; also fixed a pre-existing, unrelated staleness in the "Rotating the admin key" section, which still documented a one-step set_admin call instead of the actual two-step propose_admin/accept_admin flow already in the contract
  • scripts/testnet-smoke.sh, scripts/testnet-load.sh: deploy commands updated to pass admin as a constructor argument
  • packages/tholos-sdk: bindings regenerated from the updated contract interface
  • contracts/asserter-consumer, contracts/demo-consumer: test fixtures updated to register with the constructor argument and call the new initialize signature

Three existing tests asserted NotInitialized from calls (set_paused, update_resolvers, set_bond_amount) that only ever checked whether Admin exists in storage. Since __constructor now sets Admin atomically with deployment, that state no longer exists for any live contract, so those calls succeed instead. Rewrote the three tests to assert the new, correct behavior, and two more that used set_paused as an indirect "nothing was persisted" proof to instead retry initialize directly. Added a new regression test, test_initialize_rejects_caller_other_than_constructor_admin, mirroring v2's equivalent.

Test plan

  • cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test --workspace pass locally (all crates green, 89 tholos tests including the new regression test, 6 each for asserter-consumer and demo-consumer)
  • CONTRACT.md updated for the public interface change
  • scripts/testnet-smoke.sh not run against testnet for this change; the constructor-argument deploy shape mirrors v2's already-verified pattern
  • Manually verified: test snapshots regenerated in isolated per-crate runs and confirmed reproducible (diffed against a second isolated run) before committing, to keep unrelated non-deterministic address churn out of this PR

Note on assignment

This issue was assigned to @odiliflash-ops, who did not respond after two check-ins over several days, including an explicit 24-hour deadline. Unassigning and fixing directly here with the maintainer's go-ahead.

@collinsezedike
collinsezedike merged commit ea715a1 into main Sep 11, 2026
4 checks passed
@collinsezedike
collinsezedike deleted the fix/tholos-v1-initialize-front-running branch September 11, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] initialize is front-runnable in tholos v1

1 participant