Harden TaskBounty escrow: safe transfers, refund event amount, fix build - #136
Merged
Abd-Standard merged 2 commits intoJul 30, 2026
Merged
Conversation
The escrow flow (lock on create_task, release on approve_submission, refund on cancel_task) already existed but the crate could not compile or run its test suite, so none of it had ever actually been verified: - Error used #[contracttype] instead of #[contracterror], so panic_with_error! couldn't compile. - Storage keys used raw byte-string literals (b"TASK", ...) as tuple keys, which current soroban-sdk no longer accepts; switched to symbol_short!, matching the pattern already used in events.rs. - panic_with_error! needs an explicit import in current soroban-sdk. - Bumped soroban-sdk 21 -> 23 (matching the sibling contract crate in this repo) since 21's pinned soroban-env-host pulls an unbounded ed25519-dalek release that no longer builds; updated the one test helper (mint) whose API moved to StellarAssetClient in the process. On top of getting it building and green, hardened the escrow itself: - Token transfers (escrow lock, release, refund) now use try_transfer and surface the existing-but-unused Error::PaymentFailed instead of panicking with an opaque host error on a failed payment. - The refund event now carries the refunded amount, so deposits, releases, and refunds are all auditable on-chain with amounts. - Added tests for refund-after-deadline, the refund event's amount, and that a completed task can no longer be cancelled/refunded out from under an already-paid contributor. All 15 unit tests pass (cargo test).
Kept the escrow-safety additions from this branch (try_transfer + PaymentFailed, refund event amount, new cancel/refund tests) on top of upstream's newer task categories/tags/duplicate-detection work. Also fixed three build/test bugs already present in upstream main that this merge exposed (none introduced by this branch): - storage.rs's has_duplicate_task/set_duplicate_task used String without importing it. - test.rs called .address() on TokenClient's `address` field instead of accessing it directly, in 5 tests. - create_task moved `description` into the Task struct before also borrowing it for the duplicate-task key. - test_same_task_allowed_for_different_poster never funded the second poster before it tried to create a task, so it always failed on insufficient balance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #130
Summary
create_task, release onapprove_submission, refund oncancel_task) already existed inDocuments/Task Bounty/src/{task,submission}.rs, but the crate did not compile, so it had never actually been built or tested.Errorneeded#[contracterror](not#[contracttype]) forpanic_with_error!to work, storage keys used raw byte-string literals that currentsoroban-sdkno longer accepts as tuple keys (switched tosymbol_short!, matching the existing pattern inevents.rs),panic_with_error!needed an explicit import, andsoroban-sdkwas bumped from 21 → 23 (matching the siblingcontract/crate already in this repo) since 21's pinnedsoroban-env-hostpulls an unboundeded25519-dalekrelease that no longer builds against current crates.io.try_transferand surface the existing-but-previously-unusedError::PaymentFailedinstead of panicking with an opaque host error on a failed payment.Test plan
cargo test— 15/15 passingcargo build— clean (warnings only, pre-existing/unrelated to this change)