Skip to content

feat: Add get_redemption_request view functions and user query support #151

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.

Sign up for GitHub

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

Open
trinnode wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Add get_redemption_request view functions and user query support #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion soroban-contracts/contracts/single_rwa_vault/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,16 @@ impl SingleRWAVault {
get_redemption_request(e, request_id)
}

/// Returns the current redemption request counter (total number of requests ever made).
pub fn redemption_counter(e: &Env) -> u32 {
get_redemption_counter(e)
}

/// Returns all redemption request IDs for a given user.
pub fn get_user_redemption_requests(e: &Env, user: Address) -> Vec<u32> {
get_user_redemption_requests(e, &user)
}
Comment on lines +510 to +518
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New query surface (redemption_counter / get_user_redemption_requests) and the per-user tracking added in request_early_redemption don’t appear to have direct test coverage. Please add tests that (1) multiple requests by the same user produce an ordered ID list, (2) different users don’t leak IDs, and (3) redemption_counter matches the highest issued ID.

Copilot uses AI. Check for mistakes.
Comment on lines +510 to +518
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR/issue description says the contract should expose a public get_redemption_request(request_id) view, but the public interface here still only exposes redemption_request(...). If external clients are expecting the new name, consider adding get_redemption_request as a wrapper/alias (keeping redemption_request for backwards compatibility) or updating the PR description accordingly.

Copilot uses AI. Check for mistakes.

// ─────────────────────────────────────────────────────────────────
// ERC-4626 max helpers
// ─────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -1211,12 +1221,14 @@ impl SingleRWAVault {
e,
id,
RedemptionRequest {
user: caller,
user: caller.clone(),
shares,
request_time: e.ledger().timestamp(),
processed: false,
},
);
// Track the request ID in the user's list for queryability
push_user_redemption_request(e, &user, id);

Comment on lines 1228 to 1232
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The per-user request list TTL is only extended when creating a new request. However, individual RedemptionRequest entries get their TTL refreshed again on later updates (process/cancel), which can result in the request staying queryable by ID while the user’s ID list expires—breaking discovery. Consider also bumping/extending the user’s list TTL whenever a request is processed/cancelled (or choose a longer TTL strategy for the list).

Copilot uses AI. Check for mistakes.
emit_early_redemption_requested(e, user, id, shares);
bump_instance(e);
Expand Down
36 changes: 35 additions & 1 deletion soroban-contracts/contracts/single_rwa_vault/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//! INSTANCE_BUMP_AMOUNT ≈ 30 days
//! BALANCE_BUMP_AMOUNT ≈ 60 days

use soroban_sdk::{contracttype, panic_with_error, Address, Env, String};
use soroban_sdk::{contracttype, panic_with_error, Address, Env, String, Vec};

use crate::errors::Error;
use crate::types::{RedemptionRequest, Role, VaultState};
Expand Down Expand Up @@ -112,6 +112,7 @@ pub enum DataKey {
RedemptionCounter,
RedemptionRequest(u32),
EscrowedShares(Address),
// UserRedemptionRequests(Address) - removed due to contracttype size limits
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This inline note about UserRedemptionRequests(Address) being “removed due to contracttype size limits” is confusing since the enum variant isn’t present. Consider deleting the comment or replacing it with a short explanation of the chosen approach (custom key struct) and, ideally, a reference/link to the relevant size-limit context.

Suggested change
// UserRedemptionRequests(Address) - removed due to contracttype size limits
/// Per-user redemption requests are tracked via `RedemptionRequest(u32)` IDs
/// and `EscrowedShares(Address)`, instead of a dedicated
/// `UserRedemptionRequests(Address)` enum variant, to keep this `contracttype`
/// within Soroban’s data layout / size limits (see Soroban contract data docs).

Copilot uses AI. Check for mistakes.

// --- Blacklist ---
Blacklisted(Address),
Expand Down Expand Up @@ -633,6 +634,39 @@ pub fn put_escrowed_shares(e: &Env, addr: &Address, amount: i128) {
.extend_ttl(&key, BALANCE_LIFETIME_THRESHOLD, BALANCE_BUMP_AMOUNT);
}

// ─────────────────────────────────────────────────────────────────────────────
// User redemption request tracking (persistent)
// Uses a custom key struct to avoid adding to DataKey enum
// ─────────────────────────────────────────────────────────────────────────────

/// Custom storage key for per-user redemption request lists.
/// This avoids adding to the DataKey enum which has reached size limits.
#[contracttype]
#[derive(Clone)]
pub struct UserRedemptionKey {
pub user: Address,
}
Comment on lines +642 to +648
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a #[contracttype] struct-as-key with a single user field serializes as a map and has no explicit namespace/prefix, which is more storage/CPU heavy than necessary and can collide with other similarly-shaped keys in the future. Consider using a small namespaced key (e.g., a tuple with a fixed tag + Address) or adding a discriminant field to the struct to ensure uniqueness and reduce overhead.

Copilot uses AI. Check for mistakes.

/// Returns the list of redemption request IDs for a given user.
pub fn get_user_redemption_requests(e: &Env, user: &Address) -> Vec<u32> {
let key = UserRedemptionKey { user: user.clone() };
e.storage()
.persistent()
.get(&key)
.unwrap_or(Vec::new(e))
}

/// Appends a new redemption request ID to the user's list.
pub fn push_user_redemption_request(e: &Env, user: &Address, request_id: u32) {
let key = UserRedemptionKey { user: user.clone() };
let mut requests = get_user_redemption_requests(e, user);
requests.push_back(request_id);
e.storage().persistent().set(&key, &requests);
e.storage()
.persistent()
.extend_ttl(&key, BALANCE_LIFETIME_THRESHOLD, BALANCE_BUMP_AMOUNT);
}

// ─────────────────────────────────────────────────────────────────────────────
// Transfer KYC gate (instance storage)
// ─────────────────────────────────────────────────────────────────────────────
Expand Down
Loading