-
Notifications
You must be signed in to change notification settings - Fork 28
feat: implement memo-based withdrawals in escrow contract #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| [package] | ||
| name = "escrow" | ||
| version = "0.0.0" | ||
| edition = "2021" | ||
| publish = false | ||
|
|
||
| [lib] | ||
| crate-type = ["cdylib"] | ||
|
|
||
| [dependencies] | ||
| soroban-sdk = { workspace = true } | ||
| soroban-token-sdk = { workspace = true } | ||
|
|
||
| [dev-dependencies] | ||
| soroban-sdk = { workspace = true, features = ["testutils"] } | ||
| soroban-token-sdk = { workspace = true } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,210 @@ | ||
| use soroban_sdk::{contract, contractimpl, Address, BytesN, Env, Map, String, Symbol, Val, Vec}; | ||
|
|
||
| use crate::core::{DisputeManager, EscrowManager, MilestoneManager}; | ||
| use crate::error::ContractError; | ||
| use crate::events::handler::{ | ||
| ChgEsc, DisEsc, DisputeResolved, EscrowDisputed, ExtTtlEvt, FundEsc, InitEsc, | ||
| MilestoneApproved, MilestoneStatusChanged, WithdrawMemo, | ||
| }; | ||
| use crate::storage::types::{AddressBalance, Escrow}; | ||
|
|
||
| #[contract] | ||
| pub struct EscrowContract; | ||
|
|
||
| #[contractimpl] | ||
| impl EscrowContract { | ||
| pub fn __constructor() {} | ||
|
|
||
| pub fn deploy( | ||
| env: Env, | ||
| deployer: Address, | ||
| wasm_hash: BytesN<32>, | ||
| salt: BytesN<32>, | ||
| init_fn: Symbol, | ||
| init_args: Vec<Val>, | ||
| constructor_args: Vec<Val>, | ||
| ) -> (Address, Val) { | ||
| if deployer != env.current_contract_address() { | ||
| deployer.require_auth(); | ||
| } | ||
|
|
||
| let deployed_address = env | ||
| .deployer() | ||
| .with_address(deployer, salt) | ||
| .deploy_v2(wasm_hash, constructor_args); | ||
|
|
||
| let res: Val = env.invoke_contract(&deployed_address, &init_fn, init_args); | ||
| (deployed_address, res) | ||
| } | ||
|
|
||
| //////////////////////// | ||
| // Escrow ///// | ||
| //////////////////////// | ||
|
|
||
| pub fn initialize_escrow(e: &Env, escrow_properties: Escrow) -> Result<Escrow, ContractError> { | ||
| let initialized_escrow = EscrowManager::initialize_escrow(e, escrow_properties)?; | ||
| InitEsc { | ||
| escrow: initialized_escrow.clone(), | ||
| } | ||
| .publish(e); | ||
| Ok(initialized_escrow) | ||
| } | ||
|
|
||
| pub fn fund_escrow( | ||
| e: &Env, | ||
| signer: Address, | ||
| expected_escrow: Escrow, | ||
| amount: i128, | ||
| ) -> Result<(), ContractError> { | ||
| EscrowManager::fund_escrow(e, &signer, &expected_escrow, amount)?; | ||
| FundEsc { signer, amount }.publish(e); | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn release_funds( | ||
| e: &Env, | ||
| release_signer: Address, | ||
| trustless_work_address: Address, | ||
| ) -> Result<(), ContractError> { | ||
| EscrowManager::release_funds(e, &release_signer, &trustless_work_address)?; | ||
| DisEsc { release_signer }.publish(e); | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn withdraw_with_memo(e: &Env) -> Result<(), ContractError> { | ||
| let escrow = EscrowManager::get_escrow(e)?; | ||
| EscrowManager::withdraw_with_memo(e, &e.current_contract_address())?; | ||
| WithdrawMemo { | ||
| amount: escrow.withdrawal_state.pending_receiver_amount, | ||
| receiver: escrow.roles.receiver, | ||
| memo: escrow.receiver_memo, | ||
| } | ||
| .publish(e); | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn update_escrow( | ||
| e: &Env, | ||
| plataform_address: Address, | ||
| escrow_properties: Escrow, | ||
| ) -> Result<Escrow, ContractError> { | ||
| let updated_escrow = EscrowManager::change_escrow_properties( | ||
| e, | ||
| &plataform_address, | ||
| escrow_properties.clone(), | ||
| )?; | ||
| ChgEsc { | ||
| platform: plataform_address, | ||
| engagement_id: escrow_properties.engagement_id.clone(), | ||
| } | ||
| .publish(e); | ||
| Ok(updated_escrow) | ||
| } | ||
|
Comment on lines
+86
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chainTypo: plataform_address → platform_address (public API). Public API misspelling propagates across clients/tests. - pub fn update_escrow(
- e: &Env,
- plataform_address: Address,
- escrow_properties: Escrow,
- ) -> Result<Escrow, ContractError> {
- let updated_escrow = EscrowManager::change_escrow_properties(
- e,
- &plataform_address,
- escrow_properties.clone(),
- )?;
- ChgEsc {
- platform: plataform_address,
+ pub fn update_escrow(
+ e: &Env,
+ platform_address: Address,
+ escrow_properties: Escrow,
+ ) -> Result<Escrow, ContractError> {
+ let updated_escrow = EscrowManager::change_escrow_properties(
+ e,
+ &platform_address,
+ escrow_properties.clone(),
+ )?;
+ ChgEsc {
+ platform: platform_address,
engagement_id: escrow_properties.engagement_id.clone(),
}
.publish(e);
Ok(updated_escrow)
}Run to find other occurrences: 🏁 Script executed: #!/bin/bash
rg -nP 'plataform_address|Plataform' -C2Length of output: 6034 Rename all occurrences of
🤖 Prompt for AI Agents |
||
|
|
||
| pub fn get_escrow(e: &Env) -> Result<Escrow, ContractError> { | ||
| EscrowManager::get_escrow(e) | ||
| } | ||
|
|
||
| pub fn get_escrow_by_contract_id( | ||
| e: &Env, | ||
| contract_id: Address, | ||
| ) -> Result<Escrow, ContractError> { | ||
| EscrowManager::get_escrow_by_contract_id(e, &contract_id) | ||
| } | ||
|
|
||
| pub fn get_multiple_escrow_balances( | ||
| e: &Env, | ||
| addresses: Vec<Address>, | ||
| ) -> Result<Vec<AddressBalance>, ContractError> { | ||
| EscrowManager::get_multiple_escrow_balances(e, addresses) | ||
| } | ||
|
|
||
| //////////////////////// | ||
| // Admin / TTL ///// | ||
| //////////////////////// | ||
|
|
||
| pub fn extend_contract_ttl( | ||
| e: &Env, | ||
| platform_address: Address, | ||
| ledgers_to_extend: u32, | ||
| ) -> Result<(), ContractError> { | ||
| platform_address.require_auth(); | ||
|
|
||
| let escrow = EscrowManager::get_escrow(e)?; | ||
| if platform_address != escrow.roles.platform_address { | ||
| return Err(ContractError::OnlyPlatformAddressExecuteThisFunction); | ||
| } | ||
|
|
||
| let min_ledgers = 1u32; | ||
| e.storage() | ||
| .instance() | ||
| .extend_ttl(min_ledgers, ledgers_to_extend); | ||
|
|
||
| ExtTtlEvt { | ||
| platform: platform_address, | ||
| ledgers_to_extend, | ||
| } | ||
| .publish(e); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| //////////////////////// | ||
| // Milestones ///// | ||
| //////////////////////// | ||
|
|
||
| pub fn change_milestone_status( | ||
| e: Env, | ||
| milestone_index: i128, | ||
| new_status: String, | ||
| new_evidence: Option<String>, | ||
| service_provider: Address, | ||
| ) -> Result<(), ContractError> { | ||
| let escrow = MilestoneManager::change_milestone_status( | ||
| &e, | ||
| milestone_index, | ||
| new_status, | ||
| new_evidence, | ||
| service_provider, | ||
| )?; | ||
| MilestoneStatusChanged { escrow }.publish(&e); | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn approve_milestone( | ||
| e: Env, | ||
| milestone_index: i128, | ||
| approver: Address, | ||
| ) -> Result<(), ContractError> { | ||
| let escrow = | ||
| MilestoneManager::change_milestone_approved_flag(&e, milestone_index, approver)?; | ||
| MilestoneApproved { escrow }.publish(&e); | ||
| Ok(()) | ||
| } | ||
|
|
||
| //////////////////////// | ||
| // Disputes ///// | ||
| //////////////////////// | ||
|
|
||
| pub fn resolve_dispute( | ||
| e: Env, | ||
| dispute_resolver: Address, | ||
| trustless_work_address: Address, | ||
| distributions: Map<Address, i128>, | ||
| ) -> Result<(), ContractError> { | ||
| let escrow = DisputeManager::resolve_dispute( | ||
| &e, | ||
| dispute_resolver, | ||
| trustless_work_address, | ||
| distributions, | ||
| )?; | ||
| DisputeResolved { escrow }.publish(&e); | ||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn dispute_escrow(e: Env, signer: Address) -> Result<(), ContractError> { | ||
| let escrow = DisputeManager::dispute_escrow(&e, signer)?; | ||
| EscrowDisputed { escrow }.publish(&e); | ||
| Ok(()) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| use soroban_sdk::token::Client as TokenClient; | ||
| use soroban_sdk::{Address, Env, Map}; | ||
|
|
||
| use crate::core::escrow::EscrowManager; | ||
| use crate::error::ContractError; | ||
| use crate::modules::{ | ||
| fee::{FeeCalculator, FeeCalculatorTrait}, | ||
| math::{BasicArithmetic, BasicMath}, | ||
| }; | ||
|
Comment on lines
+6
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix rounding/overdraft and overflow in fee pro‑rata distribution.
Apply: @@
-use crate::modules::{
- fee::{FeeCalculator, FeeCalculatorTrait},
- math::{BasicArithmetic, BasicMath},
-};
+use crate::modules::{
+ fee::{FeeCalculator, FeeCalculatorTrait},
+ math::{BasicArithmetic, BasicMath, SafeMath},
+};
@@
- let fee_result = FeeCalculator::calculate_standard_fees(total, escrow.platform_fee)?;
- let total_fees =
- BasicMath::safe_add(fee_result.trustless_work_fee, fee_result.platform_fee)?;
+ let fee_result = FeeCalculator::calculate_standard_fees(total, escrow.platform_fee)?;
+ let total_fees =
+ BasicMath::safe_add(fee_result.trustless_work_fee, fee_result.platform_fee)?;
+ let net_pot = fee_result.receiver_amount;
@@
- for (addr, amount) in distributions.iter() {
- if amount <= 0 {
- continue;
- }
- let fee_share = (amount * (total_fees as i128)) / total;
- let net_amount = amount - fee_share;
- if net_amount > 0 {
- token_client.transfer(&contract_address, &addr, &net_amount);
- }
- }
+ // Pro‑rata net distribution with deterministic remainder allocation
+ let mut assigned: i128 = 0;
+ let mut last_addr: Option<Address> = None;
+ for (addr, amount) in distributions.iter() {
+ if amount <= 0 {
+ continue;
+ }
+ last_addr = Some(addr.clone());
+ let share = SafeMath::safe_mul_div(amount, net_pot, total)?;
+ if share > 0 {
+ token_client.transfer(&contract_address, &addr, &share);
+ assigned = BasicMath::safe_add(assigned, share)?;
+ }
+ }
+ let remainder = BasicMath::safe_sub(net_pot, assigned)?;
+ if remainder > 0 {
+ if let Some(addr) = last_addr {
+ token_client.transfer(&contract_address, &addr, &remainder);
+ }
+ }Rationale anchored by existing primitives (SafeMath::safe_mul_div, BasicMath). Based on provided snippets. Also applies to: 47-50, 66-75 🤖 Prompt for AI Agents |
||
| use crate::storage::types::{DataKey, Escrow}; | ||
|
|
||
| use super::validators::dispute::{ | ||
| validate_dispute_flag_change_conditions, validate_dispute_resolution_conditions, | ||
| }; | ||
|
|
||
| pub struct DisputeManager; | ||
|
|
||
| impl DisputeManager { | ||
| pub fn resolve_dispute( | ||
| e: &Env, | ||
| dispute_resolver: Address, | ||
| trustless_work_address: Address, | ||
| distributions: Map<Address, i128>, | ||
| ) -> Result<Escrow, ContractError> { | ||
| dispute_resolver.require_auth(); | ||
| let mut escrow = EscrowManager::get_escrow(e)?; | ||
| let contract_address = e.current_contract_address(); | ||
|
|
||
| let token_client = TokenClient::new(&e, &escrow.trustline.address); | ||
| let current_balance = token_client.balance(&contract_address); | ||
|
|
||
| let mut total: i128 = 0; | ||
| for (_addr, amount) in distributions.iter() { | ||
| if amount <= 0 { | ||
| return Err(ContractError::AmountsToBeTransferredShouldBePositive); | ||
| } | ||
| total = BasicMath::safe_add(total, amount)?; | ||
| } | ||
|
|
||
| validate_dispute_resolution_conditions( | ||
| &escrow, | ||
| &dispute_resolver, | ||
| current_balance, | ||
| total, | ||
| )?; | ||
|
|
||
| let fee_result = FeeCalculator::calculate_standard_fees(total, escrow.platform_fee)?; | ||
| let total_fees = | ||
| BasicMath::safe_add(fee_result.trustless_work_fee, fee_result.platform_fee)?; | ||
|
|
||
| if fee_result.trustless_work_fee > 0 { | ||
| token_client.transfer( | ||
| &contract_address, | ||
| &trustless_work_address, | ||
| &fee_result.trustless_work_fee, | ||
| ); | ||
| } | ||
|
Comment on lines
+51
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainValidate the trustless_work fee recipient; do not take it from the caller.
Follow‑up: If you already store TW address elsewhere, please wire it here and reject mismatches. Would you like me to draft a storage key (e.g., DataKey::TrustlessWorkAddress) change and migration? 🏁 Script executed: #!/bin/bash
# Locate and display dispute.rs context
FILE=$(fd --hidden 'dispute.rs' -t f | head -n1)
sed -n '1,100p' "$FILE"
# Search all occurrences of trustless_work_address
rg -n 'trustless_work_address' -C3 .Length of output: 46561 🏁 Script executed: rg -n "enum DataKey" -C3 "Timebased Escrows/contracts/escrow/src/storage/types.rs"Length of output: 98 🏁 Script executed: #!/bin/bash
rg -n "pub struct Escrow" -C3 "Timebased Escrows/contracts/escrow/src/core/escrow.rs"Length of output: 206 Enforce a configured |
||
| if fee_result.platform_fee > 0 { | ||
| token_client.transfer( | ||
| &contract_address, | ||
| &escrow.roles.platform_address, | ||
| &fee_result.platform_fee, | ||
| ); | ||
| } | ||
|
|
||
| for (addr, amount) in distributions.iter() { | ||
| if amount <= 0 { | ||
| continue; | ||
| } | ||
| let fee_share = (amount * (total_fees as i128)) / total; | ||
| let net_amount = amount - fee_share; | ||
| if net_amount > 0 { | ||
| token_client.transfer(&contract_address, &addr, &net_amount); | ||
| } | ||
| } | ||
|
|
||
| escrow.flags.resolved = true; | ||
| escrow.flags.disputed = false; | ||
| e.storage().instance().set(&DataKey::Escrow, &escrow); | ||
|
|
||
| Ok(escrow) | ||
| } | ||
|
|
||
| pub fn dispute_escrow(e: &Env, signer: Address) -> Result<Escrow, ContractError> { | ||
| signer.require_auth(); | ||
| let mut escrow = EscrowManager::get_escrow(e)?; | ||
| validate_dispute_flag_change_conditions(&escrow, &signer)?; | ||
|
|
||
| escrow.flags.disputed = true; | ||
| e.storage().instance().set(&DataKey::Escrow, &escrow); | ||
|
|
||
| Ok(escrow) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Memo verification path unclear; function appears permissionless.
Where is the check that the transaction’s memo matches the stored receiver memo? The contract method doesn’t accept a memo parameter and I don’t see a call that reads an L1 memo from Env.
If on-chain memo access isn’t supported, the contract cannot verify memos; enforcement must live in the client/backend. In that case, update docs/tests to reflect “memo required by client, not verified on-chain.”
Authorization: anyone can call withdraw_with_memo here. Confirm EscrowManager::withdraw_with_memo enforces:
If not already enforced, add these checks in the manager/validators.
🌐 Web query:
💡 Result:
Short answer: No — a Soroban contract cannot directly read the Stellar transaction memo from env (there is no Env API that returns the transaction memo). The memo can be used in Soroban authorization credentials (CAP‑0064 adds a txMemo field for SorobanAddressCredentialsV2 and the host validates it), but that memo-checking happens in the host/authorization layer, not by the contract code via soroban_sdk::Env. [1][2][3]
Sources
References
Length of output: 1055
🏁 Script executed:
Length of output: 1215
🏁 Script executed:
Length of output: 1057
🏁 Script executed:
Length of output: 4321
Cannot verify tx memo on-chain; enforce off-chain and restrict withdrawal to receiver
Soroban Env has no API for reading the Stellar transaction memo—memo validation must happen in your client/backend and update docs/tests to reflect “memo required off-chain, not verified on-chain.” The existing
validate_withdrawal_conditionsalready enforcesfees_released,pending_receiver_amount > 0, non-empty stored memo, idempotency, and state reset. To ensure only the designated receiver can trigger withdrawal, add anEnv::caller() == escrow.roles.receivercheck inwithdraw_with_memo.