feat(escrow): add read-only EscrowSummary view#225
Conversation
- Add EscrowSummary contracttype with escrow_id, buyer, seller, amount, status - Add get_escrow_summary() returning Result<EscrowSummary, EscrowError::NotFound> - Add unit tests for missing and existing escrow ids Closes DelegoLabs#90
|
@Mikkelnice 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! 🚀 |
📝 WalkthroughWalkthroughAdds a compact ChangesEscrow summary view
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contracts/escrow/src/test.rs`:
- Around line 50-52: The test setup still uses the deprecated
register_stellar_asset_contract API when creating the Stellar asset token.
Update the token registration in the escrow test to use
register_stellar_asset_contract_v2 instead, and obtain the contract address from
the returned handle before calling StellarAssetClient::new and minting to the
buyer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: acc318cf-c62b-438b-8807-5212c3b99a0d
📒 Files selected for processing (2)
contracts/escrow/src/lib.rscontracts/escrow/src/test.rs
| let token_admin = Address::generate(&env); | ||
| let token_id = env.register_stellar_asset_contract(token_admin.clone()); | ||
| soroban_sdk::token::StellarAssetClient::new(&env, &token_id).mint(&buyer, &10000); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
Is Env::register_stellar_asset_contract deprecated in soroban-sdk 22.0.0, and what is the recommended replacement?
💡 Result:
Yes, the function register_stellar_asset_contract is deprecated in soroban-sdk version 22.0.0 (and later versions) [1]. It is marked as deprecated within the testutils module of the Env struct [1]. The recommended replacement is register_stellar_asset_contract_v2 [1][2]. Unlike the original version, which returned only the contract ID [1], register_stellar_asset_contract_v2 returns a utility struct that includes the contract ID and provides additional methods for reading and updating issuer flags (such as AUTH_REVOCABLE and AUTH_REQUIRED) [1][2]. This updated function is specifically designed to facilitate testing scenarios that require interaction with Stellar Asset Contract (SAC) configuration [1][2][3].
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/struct.Env.html
- 2: https://developers.stellar.org/docs/build/guides/tokens/stellar-asset-contract
- 3: Expose more customizable SAC deployment functions stellar/rs-soroban-sdk#1117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the test file and dependency declarations related to soroban-sdk.
git ls-files | rg '(^|/)Cargo\.toml$|contracts/escrow/src/test\.rs$'
echo '---'
for f in $(git ls-files | rg '(^|/)Cargo\.toml$|contracts/escrow/src/test\.rs$'); do
echo "### $f"
sed -n '1,220p' "$f"
echo
doneRepository: DelegoLabs/Delego
Length of output: 4743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the test file around the reported lines.
sed -n '1,120p' contracts/escrow/src/test.rsRepository: DelegoLabs/Delego
Length of output: 2774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the test file and dependency declarations related to soroban-sdk.
git ls-files | rg '(^|/)Cargo\.toml$|contracts/escrow/src/test\.rs$'
echo '---'
for f in $(git ls-files | rg '(^|/)Cargo\.toml$|contracts/escrow/src/test\.rs$'); do
echo "### $f"
sed -n '1,220p' "$f"
echo
doneRepository: DelegoLabs/Delego
Length of output: 4743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the test file around the reported lines.
sed -n '1,120p' contracts/escrow/src/test.rsRepository: DelegoLabs/Delego
Length of output: 2774
Use register_stellar_asset_contract_v2 here
register_stellar_asset_contract is deprecated in soroban-sdk 22.0.0. Switch this test to register_stellar_asset_contract_v2 and take the address from the returned contract handle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contracts/escrow/src/test.rs` around lines 50 - 52, The test setup still uses
the deprecated register_stellar_asset_contract API when creating the Stellar
asset token. Update the token registration in the escrow test to use
register_stellar_asset_contract_v2 instead, and obtain the contract address from
the returned handle before calling StellarAssetClient::new and minting to the
buyer.
1 participant
Summary
Adds a compact getter for API/indexer consumers that need all escrow display fields in one call.
Changes
EscrowSummarycontracttype:escrow_id,buyer,seller,amount,statusget_escrow_summary(escrow_id)→Result<EscrowSummary, EscrowError>— returnsNotFoundfor missing escrowsNotFound; existing escrow returns correct summary fieldsFiles Changed
contracts/escrow/src/lib.rscontracts/escrow/src/test.rsCloses #90
Summary by CodeRabbit
New Features
Tests