fix: repair corrupted admin.rs allowlist/timelock functions, fix script shebangs

[david87131]

Problem

contracts/escrow/src/admin.rs was corrupted by a bad merge (introduced in #702): several new functions were added with their opening signature but no body/closing brace, causing their doc-comment + signature to be immediately followed by the next function's pub fn, and in a couple of cases an unrelated function's leftover body ended up glued onto the wrong signature. The result does not compile.

What was wrong, and how each piece was repaired

• set_token_allowlist_enabled (admin.rs corrupted — set_token_allowlist_enabled has no closing brace, functions overlapping #710): had no closing brace — its auth-check body fell straight into get_allowed_tokens. Restored so it persists DataKey::TokenAllowlistEnabled and emits allowlist_toggled, with its own closing brace.

• get_treasury (admin.rs corrupted — set_token_allowlist_enabled has no closing brace, functions overlapping #710): returned read_treasury(&env) but had no closing brace before the // 7. SetFeeCollector block started. Added the missing }.

• is_action_paused: its body was overwritten mid-function by fee-collector-setting logic (.set(&DataKey::FeeCollector, ...) / Ok(())), while the real tail of is_action_paused (.get(&DataKey::ActionPaused(action)).unwrap_or(false)) was orphaned several lines below, itself followed by a doc comment for get_arbitration_fee. Restored is_action_paused to a single, complete body, and reconstructed the missing set_fee_collector(env, new_collector) direct admin function from the fragments — signature/behavior verified against test_set_fee_collector.rs and test_auth_matrix.rs::set_fee_collector_requires_admin_auth, which expect it to take only new_collector (no caller param), gate via admin.require_auth(), reject the zero address with InvalidAddress, and emit emit_fee_collector_updated.

• add_allowed_token (admin.rs corrupted — remove_allowed_token body replaced with cancel_timelock_op logic #711): its final expression read DataKey::ResolverStrict and returned a bool, not Result<(), ContractError>. Fixed to push the token into the Vec<Address> stored at DataKey::TokenAllowlist and return Ok(()), matching the pattern used by execute_add_allowed_token.

• remove_allowed_token (admin.rs corrupted — remove_allowed_token body replaced with cancel_timelock_op logic #711): had an empty body that fell straight into what was labeled cancel_timelock_op. That body actually implemented "read TokenAllowlist, check membership, remove, emit token_allowlist_updated(token, false)" and referenced a token variable cancel_timelock_op never declared — clear evidence it belonged to remove_allowed_token. Moved that logic into remove_allowed_token (rewritten against Vec<Address> to match add_allowed_token/execute_add_allowed_token/execute_remove_allowed_token's storage format instead of the Map<Address,bool> it was using).

• cancel_timelock_op (admin.rs corrupted — remove_allowed_token body replaced with cancel_timelock_op logic #711): now implements real timelock cancellation using the existing DataKey::TimelockOp(u32) key (the same key queue_timelock_op/execute_timelock_op already use): loads the queued proposal via storage::read_timelock_proposal, returns ContractError::InvalidState if nothing is queued for that operation, removes it via storage::remove_timelock_proposal, and emits emit_timelock_cancelled(env, operation, proposal.proposer, caller).

• Removed duplicate definitions the same bad merge left behind, which are duplicate-item compile errors once the file is otherwise fixed:

  • a second is_token_allowlist_enabled,
  • a second get_allowed_tokens that read DataKey::TokenAllowlist as a Map<Address, bool> and returned .keys() — inconsistent with the Vec<Address> format used everywhere else (add_allowed_token, remove_allowed_token, queue_add_allowed_token/execute_add_allowed_token, queue_remove_allowed_token/execute_remove_allowed_token); kept the Vec<Address>-based copy for consistency,
  • a second get_platform_fee_bps.

• Shebangs (Fix shebang portability in build.sh and scripts/deploy.sh #622): build.sh and scripts/deploy.sh used #!/bin/bash (a fixed path that doesn't exist on NixOS/BSDs/some custom setups). Changed both to #!/usr/bin/env bash, matching the other 8 scripts in the repo.

Out of scope (found, not touched)

While verifying, I found contracts/escrow/src/instructions.rs (and a related duplicate ContractError::InvalidMulticallArg variant in errors.rs) has its own, unrelated pre-existing corruption from the same bad merge — a duplicated/misplaced multicall signature and a matching missing brace — which currently blocks cargo build --lib for the whole crate independent of anything in this PR. That's a separate, larger repair and not part of issues #710/#711/#622, so I left it alone. I verified admin.rs itself compiles cleanly by temporarily patching just that instructions.rs line locally (not committed): with that one-line local patch, cargo build --lib succeeds with zero errors or warnings originating from admin.rs. On the actual committed tree, cargo build --lib fails only inside instructions.rs — no errors reference admin.rs.

Test plan

• cargo build --lib from contracts/escrow/ — no errors or warnings from admin.rs (confirmed via a local, uncommitted probe-patch of the unrelated instructions.rs issue; the committed tree's sole compile error is in instructions.rs, pre-existing and out of scope)
• bash -n build.sh — passes
• bash -n scripts/deploy.sh — passes
• Manual read-through of every function boundary in admin.rs against the TimelockOperation/DataKey enums and the existing queue_/execute_ timelocked variants to confirm house style (auth pattern, error handling, event emission) is matched

Fixes #711
Fixes #710
Fixes #622

[drips-wave]

@david87131 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits