RWA: deploy example and testnet e2e flow#654
Draft
pasevin wants to merge 3 commits intoOpenZeppelin:mainfrom
Draft
RWA: deploy example and testnet e2e flow#654pasevin wants to merge 3 commits intoOpenZeppelin:mainfrom
pasevin wants to merge 3 commits intoOpenZeppelin:mainfrom
Conversation
Transplant the reviewed deploy crates, scripts, and required module assets onto upstream/main so the deploy and testnet e2e PR can stand on its own.
Encode IRS country profiles as explicit ScVal payloads and reuse the existing invoke retry helper so the standalone deploy branch passes testnet e2e.
Contributor
WalkthroughThis pull request introduces comprehensive Real-World Assets (RWA) compliance module infrastructure for Stellar tokens, including seven new compliance modules (country allow/restrict, supply limits, transfer restrictions, initial lockup periods, max balance, time-based transfer limits), deployment contracts (token, identity registry, verifier, compliance), build and deployment automation scripts, and supporting storage layers. Changes
Sequence Diagram(s)sequenceDiagram
participant Deploy as Deployment Script
participant IRS as Identity Registry
participant Token as RWA Token
participant Compliance as Compliance Orchestrator
participant Module as Compliance Module
Deploy->>IRS: Deploy & initialize (manager)
Deploy->>Compliance: Deploy & initialize (admin)
Deploy->>Token: Deploy & initialize (compliance)
Deploy->>Module: Deploy (admin bootstrap)
Module->>Module: set_identity_registry_storage
Module->>Module: Configure module state<br/>(limits, allowlists, etc.)
Module->>Module: set_compliance_address<br/>(transfer control)
Deploy->>Compliance: Register module on hooks<br/>(CanTransfer, CanCreate, etc.)
Deploy->>Compliance: Verify hook wiring
Note over Deploy,Module: Token Operations Phase
Token->>Compliance: Transfer/Create request
Compliance->>Module: Invoke compliance hook<br/>(can_transfer/can_create)
Module->>IRS: Query identity data<br/>(country, balance, etc.)
Module->>Module: Validate against config
Module-->>Compliance: Allow/Deny decision
Compliance-->>Token: Proceed/Reject
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes This change introduces substantial new functionality across multiple interconnected subsystems: seven distinct compliance modules with custom storage, state management, and lifecycle hooks; four infrastructure deployment contracts; comprehensive automation scripts with complex orchestration logic; and supporting module traits with default implementations. The heterogeneity is significant—compliance logic differs per module (country-based, time-windowed, identity-based restrictions), storage patterns vary, and scripts contain sophisticated retry/validation logic. While individual components follow patterns established in the codebase, the aggregate scope, density of new public interfaces, cross-module dependencies, and deployment orchestration complexity demand thorough review across contract logic, storage safety, authorization gates, and integration points. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
examples/rwa-country-allow/README.md (1)
15-27: Consider adding an explicit “example-only, not production-hardening guidance” note.
Given this section explains authorization behavior, a short disclaimer would reduce the risk of downstream copy/paste into production setups without added hardening.Based on learnings: contracts under
examples/rwa-compliance/and related examples are intentionally simplified test/example infrastructure and should not be treated as production auth patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-country-allow/README.md` around lines 15 - 27, Add a short, explicit disclaimer to the Authorization model section stating this is an example-only simplified pattern and not production-ready for hardening; mention the constructor’s one-time admin, the one-time set_compliance_address flow and reliance on the bound Compliance contract are intentionally simplified, and refer readers to the examples/rwa-compliance folder for other test-only contracts and to consult production security guidance before reuse.examples/rwa-deploy/scripts/e2e.sh (2)
21-22: Shellcheck warning is a false positive.
NETWORKis used by the sourcedcommon.shhelper functions (e.g.,invoke(),invoke_readonly()). Consider adding a shellcheck directive:# shellcheck disable=SC2034 # Used by sourced common.sh NETWORK="${STELLAR_NETWORK:-testnet}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/e2e.sh` around lines 21 - 22, The shellcheck warning about NETWORK being unused is a false positive because NETWORK (set from STELLAR_NETWORK) is consumed by sourced helper functions in common.sh (e.g., invoke(), invoke_readonly()); fix by adding a shellcheck directive above the assignment to silence SC2034 (e.g., add a comment like "# shellcheck disable=SC2034 # Used by sourced common.sh") so the NETWORK="${STELLAR_NETWORK:-testnet}" line remains but the linter is suppressed with clear justification referencing common.sh and the invoke()/invoke_readonly() helpers.
346-346: Minor: Summary box alignment may break with multi-digit counts.The fixed padding assumes single-digit or small counts. If tests grow significantly, the box alignment will be off. Consider using dynamic width calculation:
♻️ Optional fix for dynamic alignment
-printf "║ Tests: %d passed, %d failed (of %d) ║\n" "$PASS" "$FAIL" "$TOTAL" +printf "║ Tests: %-3d passed, %-3d failed (of %-3d) ║\n" "$PASS" "$FAIL" "$TOTAL"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/e2e.sh` at line 346, The fixed-width printf line using PASS, FAIL, TOTAL can misalign for multi-digit counts; update the printing to compute the needed field width dynamically (e.g., determine max length among PASS/FAIL/TOTAL using string length or fmt_width=$(printf "%s\n%s\n%s" "$PASS" "$FAIL" "$TOTAL" | awk '{ if (length>m) m=length } END{print m }')) and then use a dynamic width specifier in the printf call (use "%*d" with the computed width) when printing the summary so the box stays aligned regardless of digit count; locate and replace the existing printf invocation that prints PASS/FAIL/TOTAL.examples/rwa-deploy/scripts/wire.sh (1)
19-29: Shellcheck warnings are false positives.
SOURCE,NETWORK, andADMINare used by the helper functions in the sourcedcommon.shfile (e.g.,invoke()uses$SOURCEand$NETWORK;ensure_hook_registrationuses$ADMIN). Consider adding shellcheck directives to suppress these warnings for clarity:# shellcheck disable=SC2034 # Used by sourced common.sh SOURCE="${STELLAR_SOURCE:-alice}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/wire.sh` around lines 19 - 29, ShellCheck reports unused-variable warnings for SOURCE, NETWORK, and ADMIN even though they are consumed by functions in the sourced common.sh; add a ShellCheck suppression comment above each variable definition (e.g., add "# shellcheck disable=SC2034 # used by sourced common.sh" immediately before SOURCE="${STELLAR_SOURCE:-alice}", similarly for NETWORK and ADMIN) so the linter knows these vars are intentionally exported for use by functions like invoke(), ensure_hook_registration(), and read_addr() in common.sh.examples/rwa-deploy/scripts/test-happy-path.sh (1)
36-61: Consolidate retry helper to avoid policy drift.
invoke_with_retryduplicates retry logic already used in deploy scripts. Consider moving/using a single shared implementation inexamples/rwa-deploy/scripts/common.shso retry behavior changes stay consistent everywhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/test-happy-path.sh` around lines 36 - 61, The invoke_with_retry implementation in test-happy-path.sh duplicatesretry logic; extract and consolidate it into the shared helper file (common.sh) by moving the invoke_with_retry function there (including its use of STELLAR_INVOKE_RETRIES, STELLAR_INVOKE_RETRY_DELAY_SECONDS and retryable_invoke_error), then remove the duplicate from test-happy-path.sh and source common.sh at the top so the script calls the centralized invoke_with_retry; ensure retryable_invoke_error is exported or present in common.sh and preserve the same return codes and backoff behavior.examples/rwa-transfer-restrict/src/lib.rs (1)
3-3: Unused importString.The
Stringtype is imported but not used in this contract.🧹 Proposed fix to remove unused import
-use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String, Vec}; +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, Vec};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-transfer-restrict/src/lib.rs` at line 3, Remove the unused import String from the top-level use statement in lib.rs: update the import line that currently lists soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String, Vec} to omit String so only used symbols remain (contract, contractimpl, contracttype, Address, Env, Vec); ensure no other references to String exist in functions or types such as those implemented under contract/contractimpl before committing.examples/rwa-country-allow/src/lib.rs (1)
3-3: Unused importString.The
Stringtype is imported but not used in this contract.🧹 Proposed fix to remove unused import
-use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String, Vec}; +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, Vec};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-country-allow/src/lib.rs` at line 3, The import list includes an unused symbol `String` from soroban_sdk; remove `String` from the use statement that declares contract, contractimpl, contracttype, Address, Env, String, Vec so it becomes contract, contractimpl, contracttype, Address, Env, Vec to eliminate the unused import and clean up warnings.packages/tokens/src/rwa/compliance/modules/max_balance/test.rs (1)
22-208: Extract the shared Soroban mocks into a reusable test helper.
MockIRSContract,MockComplianceContract, andarm_hooks()are now near-identical here, inpackages/tokens/src/rwa/compliance/modules/initial_lockup_period/test.rs, and inpackages/tokens/src/rwa/compliance/modules/time_transfers_limits/test.rs. Keeping one shared helper will make future hook/IRS behavior changes much less likely to drift across module tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tokens/src/rwa/compliance/modules/max_balance/test.rs` around lines 22 - 208, Extract the repeated Soroban test mocks into a shared test helper module: move MockIRSContract, MockComplianceContract, and arm_hooks() into a common test-utils module and re-export their key helper methods (e.g., MockIRSContract::set_identity, MockComplianceContract::register_hook, and arm_hooks) so existing tests can call the same symbols; update the tests in TestMaxBalanceContract and the other two test files to import these helpers instead of redefining the contracts, ensuring the storage key enums (MockIRSStorageKey, MockComplianceStorageKey) and trait impls keep the same names and visibility so callers (set_identity/register_hook/arm_hooks) work unchanged.packages/tokens/src/rwa/compliance/modules/initial_lockup_period/storage.rs (1)
84-88: Remove empty/zero entries instead of persisting them.
get_locks(),get_total_locked(), andget_internal_balance()already treat missing keys as empty/0. Persisting empty lock vectors and zero balances keeps dead per-wallet state around and needlessly extends TTL/rent. Dropping those keys when the value reaches empty/0would preserve behavior and cap long-term storage growth.Also applies to: 116-120, 148-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tokens/src/rwa/compliance/modules/initial_lockup_period/storage.rs` around lines 84 - 88, The current setters persist empty vectors/zero values which keeps dead state and extends TTL; update the setter functions (e.g., set_locks, the setter handling InitialLockupStorageKey::TotalLocked, and the setter for InitialLockupStorageKey::InternalBalance) to remove the storage key when the value is empty or zero (call persistent().remove(&key)) and only call persistent().set(&key, value) and extend_ttl when the value is non-empty/non-zero so behavior of get_locks/get_total_locked/get_internal_balance (which treat missing keys as empty/0) is preserved while avoiding stale storage.examples/rwa-time-transfers-limits/src/lib.rs (1)
63-71: Consider consolidating counter read/write operations.The
increase_countersfunction callsreset_counter_if_needed(which may write the counter), then immediately reads the counter again withget_counter. This results in redundant storage operations when the counter is reset.However, this matches the pattern in the reference module implementation, so it's acceptable for this example contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-time-transfers-limits/src/lib.rs` around lines 63 - 71, The current increase_counters implementation calls reset_counter_if_needed (which may write a reset counter) and then immediately calls get_counter, causing redundant storage reads/writes; refactor so you read/update/write the counter only once by having reset_counter_if_needed return the current counter (or create a new helper like get_or_reset_counter) and then update that returned Counter and call set_counter once; update references in increase_counters to use the returned Counter from reset_counter_if_needed (or the new helper) and remove the extra get_counter call to eliminate the redundant operation.examples/rwa-deploy/scripts/common.sh (3)
20-39: Consider strengthening the contract ID validation pattern.The
is_contract_idfunction uses a basic pattern matchC[A-Z0-9]*which accepts any string starting with 'C' followed by uppercase alphanumeric characters. Stellar contract IDs are 56 characters long (C + 55 base32 characters).For this deploy script context, the current validation is likely sufficient as it catches obvious errors.
🔧 Optional: More precise validation
is_contract_id() { - case "$1" in - C[A-Z0-9]*) - return 0 - ;; - *) - return 1 - ;; - esac + local id=$1 + if [[ ${`#id`} -eq 56 && $id =~ ^C[A-Z2-7]{55}$ ]]; then + return 0 + fi + return 1 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/common.sh` around lines 20 - 39, The current is_contract_id function accepts any string starting with 'C' plus uppercase alphanumerics; tighten it to validate the full Stellar contract ID format by matching 'C' followed by exactly 55 base32 characters (uppercase A-Z and 2-7) e.g. use a pattern like ^C[A-Z2-7]{55}$ inside is_contract_id to enforce length and allowed characters, leaving require_contract_id to call is_contract_id as before; update any related tests or callers that assume the looser validation.
41-57: Inconsistent source flag usage betweeninvokeandinvoke_readonly.
invokeuses--source "$SOURCE"whileinvoke_readonlyuses--source-account "$ADMIN". The Stellar CLI accepts both--sourceand--source-accountas synonyms, so this works, but using consistent flag names would improve readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/common.sh` around lines 41 - 57, The invoke and invoke_readonly functions use inconsistent CLI flag names for the transaction sender: update one to match the other for readability (either change invoke to use --source-account "$SOURCE" or change invoke_readonly to use --source "$ADMIN"); edit the invoke function (invoke) or invoke_readonly function (invoke_readonly) so both use the same flag name (--source or --source-account) while keeping their respective variables ($SOURCE and $ADMIN) unchanged.
86-88: Consider adding error handling for missing file or invalid JSON.The
read_addrfunction will fail silently or with a cryptic Python error if$ADDR_FILEdoesn't exist or contains invalid JSON.🔧 Optional: Add validation
read_addr() { + if [ ! -f "$ADDR_FILE" ]; then + echo "ERROR: Address file not found: $ADDR_FILE" >&2 + return 1 + fi python3 -c "import json; d=json.load(open('$ADDR_FILE')); print(d$1)" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rwa-deploy/scripts/common.sh` around lines 86 - 88, The read_addr function currently calls Python directly and will fail with cryptic errors if $ADDR_FILE is missing or contains invalid JSON; update read_addr to validate that ADDR_FILE exists and is readable (e.g., check [ -f "$ADDR_FILE" ] and readable) before invoking Python, and change the Python invocation inside read_addr to wrap json.load/open and d$1 access in a try/except that prints a clear error to stderr and exits non‑zero on failure so callers can detect the error; reference the read_addr function, the ADDR_FILE variable and the Python expression d$1 when making these changes.packages/tokens/src/rwa/compliance/modules/initial_lockup_period/mod.rs (1)
38-88: Bound the lock list before it turns into a budget hotspot.
on_createdappends oneLockedTokensentry per mint, andcan_transfer/update_locked_tokensscan the full vector. That makes transfer and burn cost proportional to the wallet’s mint history, so long-lived accounts can become expensive to use. Coalescing by release timestamp and pruning fully consumed/released entries would keep runtime and storage growth predictable.Also applies to: 111-135, 206-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tokens/src/rwa/compliance/modules/initial_lockup_period/mod.rs` around lines 38 - 88, The lock vector for a wallet grows unbounded because on_created appends one LockedTokens per mint and functions like can_transfer, calculate_unlocked_amount, calculate_total_locked_amount and update_locked_tokens iterate the full Vec, making operations O(history) — fix by coalescing and pruning: when adding a lock (on_created) merge into an existing LockedTokens entry with the same release_timestamp (or nearest bucket) instead of always push_back, and in update_locked_tokens/remove paths (update_locked_tokens, can_transfer, and any setter/getter flows) drop entries with zero amount or entries with release_timestamp <= now after consuming/releasing them; additionally enforce a reasonable max buckets per (token,wallet) and read/update locks via a helper (e.g., consolidate_locks or normalize_locks) to ensure the stored Vec remains bounded and each scan processes at most that cap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/rwa-deploy/scripts/build.sh`:
- Around line 47-55: The loop that iterates over ALL and prints warnings when
WASM files under WASM_DIR are missing should instead cause the script to fail;
modify the for-loop that sets WASM_NAME and checks -f to record missing
artifacts (e.g., set a flag or increment a counter when the else branch runs)
and after the loop exit with a non-zero status (exit 1) if any missing were
detected; update the echo in the else branch to remain informative but ensure
the script returns failure by using the recorded flag/counter rather than
leaving the current warning-only behavior.
In `@packages/tokens/src/rwa/compliance/modules/max_balance/mod.rs`:
- Around line 62-65: The setter set_identity_registry_storage should reject
changing the IRS once per-identity balances exist: before calling
set_irs_address, check the IDBalance storage (e.g., iterate or query IDBalance
entries) and revert if any non-zero/any rows exist (so registry swaps are
blocked after balances are seeded); keep the existing require_auth check. Also
add the identical guard to the example contract's equivalent setter used in
pre_set_module_state so both places prevent registry swaps after balances exist.
In `@packages/tokens/src/rwa/compliance/modules/time_transfers_limits/mod.rs`:
- Around line 158-177: The removal logic in remove_time_transfer_limit leaves
per-(token,identity,limit_time) counters in storage, allowing old counters to
incorrectly influence future windows; update remove_time_transfer_limit to also
invalidate or version associated counters when a limit is removed (or increment
a window/epoch stored with the limit). Specifically, when removing the limit
from get_limits / set_limits, either (a) delete or mark expired all counters
keyed by (token, limit_time) (and identity) or (b) add/modify a version/epoch
field on the TimeTransferLimit struct and persist that change so can_transfer
(and the counter key builder) uses the version/epoch when reading/updating
counters; change the counter key construction used in can_transfer to include
this version/epoch so re-adding a limit starts fresh.
- Around line 119-122: The set_identity_registry_storage function currently
allows swapping the IRS after counters exist; change it to make the IRS binding
immutable once counters have begun accruing by rejecting updates when any
counter exists for that token. In set_identity_registry_storage, before calling
set_irs_address, check the counters storage (via get_counter or the underlying
counters map) for the given token to detect any non-zero/initialized window
entries (or alternatively add and check an initialized flag per token when the
first counter is created); if any counters are present, return/reject the change
(require_auth remains) and only allow set_irs_address when no counters exist
yet. Ensure you reference get_counter, set_identity_registry_storage,
set_irs_address and get_compliance_address when making the change.
---
Nitpick comments:
In `@examples/rwa-country-allow/README.md`:
- Around line 15-27: Add a short, explicit disclaimer to the Authorization model
section stating this is an example-only simplified pattern and not
production-ready for hardening; mention the constructor’s one-time admin, the
one-time set_compliance_address flow and reliance on the bound Compliance
contract are intentionally simplified, and refer readers to the
examples/rwa-compliance folder for other test-only contracts and to consult
production security guidance before reuse.
In `@examples/rwa-country-allow/src/lib.rs`:
- Line 3: The import list includes an unused symbol `String` from soroban_sdk;
remove `String` from the use statement that declares contract, contractimpl,
contracttype, Address, Env, String, Vec so it becomes contract, contractimpl,
contracttype, Address, Env, Vec to eliminate the unused import and clean up
warnings.
In `@examples/rwa-deploy/scripts/common.sh`:
- Around line 20-39: The current is_contract_id function accepts any string
starting with 'C' plus uppercase alphanumerics; tighten it to validate the full
Stellar contract ID format by matching 'C' followed by exactly 55 base32
characters (uppercase A-Z and 2-7) e.g. use a pattern like ^C[A-Z2-7]{55}$
inside is_contract_id to enforce length and allowed characters, leaving
require_contract_id to call is_contract_id as before; update any related tests
or callers that assume the looser validation.
- Around line 41-57: The invoke and invoke_readonly functions use inconsistent
CLI flag names for the transaction sender: update one to match the other for
readability (either change invoke to use --source-account "$SOURCE" or change
invoke_readonly to use --source "$ADMIN"); edit the invoke function (invoke) or
invoke_readonly function (invoke_readonly) so both use the same flag name
(--source or --source-account) while keeping their respective variables ($SOURCE
and $ADMIN) unchanged.
- Around line 86-88: The read_addr function currently calls Python directly and
will fail with cryptic errors if $ADDR_FILE is missing or contains invalid JSON;
update read_addr to validate that ADDR_FILE exists and is readable (e.g., check
[ -f "$ADDR_FILE" ] and readable) before invoking Python, and change the Python
invocation inside read_addr to wrap json.load/open and d$1 access in a
try/except that prints a clear error to stderr and exits non‑zero on failure so
callers can detect the error; reference the read_addr function, the ADDR_FILE
variable and the Python expression d$1 when making these changes.
In `@examples/rwa-deploy/scripts/e2e.sh`:
- Around line 21-22: The shellcheck warning about NETWORK being unused is a
false positive because NETWORK (set from STELLAR_NETWORK) is consumed by sourced
helper functions in common.sh (e.g., invoke(), invoke_readonly()); fix by adding
a shellcheck directive above the assignment to silence SC2034 (e.g., add a
comment like "# shellcheck disable=SC2034 # Used by sourced common.sh") so the
NETWORK="${STELLAR_NETWORK:-testnet}" line remains but the linter is suppressed
with clear justification referencing common.sh and the
invoke()/invoke_readonly() helpers.
- Line 346: The fixed-width printf line using PASS, FAIL, TOTAL can misalign for
multi-digit counts; update the printing to compute the needed field width
dynamically (e.g., determine max length among PASS/FAIL/TOTAL using string
length or fmt_width=$(printf "%s\n%s\n%s" "$PASS" "$FAIL" "$TOTAL" | awk '{ if
(length>m) m=length } END{print m }')) and then use a dynamic width specifier in
the printf call (use "%*d" with the computed width) when printing the summary so
the box stays aligned regardless of digit count; locate and replace the existing
printf invocation that prints PASS/FAIL/TOTAL.
In `@examples/rwa-deploy/scripts/test-happy-path.sh`:
- Around line 36-61: The invoke_with_retry implementation in test-happy-path.sh
duplicatesretry logic; extract and consolidate it into the shared helper file
(common.sh) by moving the invoke_with_retry function there (including its use of
STELLAR_INVOKE_RETRIES, STELLAR_INVOKE_RETRY_DELAY_SECONDS and
retryable_invoke_error), then remove the duplicate from test-happy-path.sh and
source common.sh at the top so the script calls the centralized
invoke_with_retry; ensure retryable_invoke_error is exported or present in
common.sh and preserve the same return codes and backoff behavior.
In `@examples/rwa-deploy/scripts/wire.sh`:
- Around line 19-29: ShellCheck reports unused-variable warnings for SOURCE,
NETWORK, and ADMIN even though they are consumed by functions in the sourced
common.sh; add a ShellCheck suppression comment above each variable definition
(e.g., add "# shellcheck disable=SC2034 # used by sourced common.sh"
immediately before SOURCE="${STELLAR_SOURCE:-alice}", similarly for NETWORK and
ADMIN) so the linter knows these vars are intentionally exported for use by
functions like invoke(), ensure_hook_registration(), and read_addr() in
common.sh.
In `@examples/rwa-time-transfers-limits/src/lib.rs`:
- Around line 63-71: The current increase_counters implementation calls
reset_counter_if_needed (which may write a reset counter) and then immediately
calls get_counter, causing redundant storage reads/writes; refactor so you
read/update/write the counter only once by having reset_counter_if_needed return
the current counter (or create a new helper like get_or_reset_counter) and then
update that returned Counter and call set_counter once; update references in
increase_counters to use the returned Counter from reset_counter_if_needed (or
the new helper) and remove the extra get_counter call to eliminate the redundant
operation.
In `@examples/rwa-transfer-restrict/src/lib.rs`:
- Line 3: Remove the unused import String from the top-level use statement in
lib.rs: update the import line that currently lists soroban_sdk::{contract,
contractimpl, contracttype, Address, Env, String, Vec} to omit String so only
used symbols remain (contract, contractimpl, contracttype, Address, Env, Vec);
ensure no other references to String exist in functions or types such as those
implemented under contract/contractimpl before committing.
In `@packages/tokens/src/rwa/compliance/modules/initial_lockup_period/mod.rs`:
- Around line 38-88: The lock vector for a wallet grows unbounded because
on_created appends one LockedTokens per mint and functions like can_transfer,
calculate_unlocked_amount, calculate_total_locked_amount and
update_locked_tokens iterate the full Vec, making operations O(history) — fix by
coalescing and pruning: when adding a lock (on_created) merge into an existing
LockedTokens entry with the same release_timestamp (or nearest bucket) instead
of always push_back, and in update_locked_tokens/remove paths
(update_locked_tokens, can_transfer, and any setter/getter flows) drop entries
with zero amount or entries with release_timestamp <= now after
consuming/releasing them; additionally enforce a reasonable max buckets per
(token,wallet) and read/update locks via a helper (e.g., consolidate_locks or
normalize_locks) to ensure the stored Vec remains bounded and each scan
processes at most that cap.
In `@packages/tokens/src/rwa/compliance/modules/initial_lockup_period/storage.rs`:
- Around line 84-88: The current setters persist empty vectors/zero values which
keeps dead state and extends TTL; update the setter functions (e.g., set_locks,
the setter handling InitialLockupStorageKey::TotalLocked, and the setter for
InitialLockupStorageKey::InternalBalance) to remove the storage key when the
value is empty or zero (call persistent().remove(&key)) and only call
persistent().set(&key, value) and extend_ttl when the value is
non-empty/non-zero so behavior of
get_locks/get_total_locked/get_internal_balance (which treat missing keys as
empty/0) is preserved while avoiding stale storage.
In `@packages/tokens/src/rwa/compliance/modules/max_balance/test.rs`:
- Around line 22-208: Extract the repeated Soroban test mocks into a shared test
helper module: move MockIRSContract, MockComplianceContract, and arm_hooks()
into a common test-utils module and re-export their key helper methods (e.g.,
MockIRSContract::set_identity, MockComplianceContract::register_hook, and
arm_hooks) so existing tests can call the same symbols; update the tests in
TestMaxBalanceContract and the other two test files to import these helpers
instead of redefining the contracts, ensuring the storage key enums
(MockIRSStorageKey, MockComplianceStorageKey) and trait impls keep the same
names and visibility so callers (set_identity/register_hook/arm_hooks) work
unchanged.
🪄 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
Run ID: 67986632-1a53-40a0-91a1-ac25e4964d5c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (60)
.gitignoreCargo.tomlexamples/rwa-country-allow/Cargo.tomlexamples/rwa-country-allow/README.mdexamples/rwa-country-allow/src/lib.rsexamples/rwa-country-restrict/Cargo.tomlexamples/rwa-country-restrict/README.mdexamples/rwa-country-restrict/src/lib.rsexamples/rwa-deploy/README.mdexamples/rwa-deploy/compliance/Cargo.tomlexamples/rwa-deploy/compliance/src/lib.rsexamples/rwa-deploy/irs/Cargo.tomlexamples/rwa-deploy/irs/src/lib.rsexamples/rwa-deploy/scripts/build-module.shexamples/rwa-deploy/scripts/build.shexamples/rwa-deploy/scripts/common.shexamples/rwa-deploy/scripts/deploy-module.shexamples/rwa-deploy/scripts/deploy.shexamples/rwa-deploy/scripts/e2e.shexamples/rwa-deploy/scripts/test-happy-path.shexamples/rwa-deploy/scripts/wire.shexamples/rwa-deploy/token/Cargo.tomlexamples/rwa-deploy/token/src/lib.rsexamples/rwa-deploy/verifier/Cargo.tomlexamples/rwa-deploy/verifier/src/lib.rsexamples/rwa-initial-lockup-period/Cargo.tomlexamples/rwa-initial-lockup-period/README.mdexamples/rwa-initial-lockup-period/src/lib.rsexamples/rwa-max-balance/Cargo.tomlexamples/rwa-max-balance/README.mdexamples/rwa-max-balance/src/lib.rsexamples/rwa-supply-limit/Cargo.tomlexamples/rwa-supply-limit/README.mdexamples/rwa-supply-limit/src/lib.rsexamples/rwa-time-transfers-limits/Cargo.tomlexamples/rwa-time-transfers-limits/README.mdexamples/rwa-time-transfers-limits/src/lib.rsexamples/rwa-transfer-restrict/Cargo.tomlexamples/rwa-transfer-restrict/README.mdexamples/rwa-transfer-restrict/src/lib.rspackages/tokens/src/rwa/compliance/modules/country_allow/mod.rspackages/tokens/src/rwa/compliance/modules/country_allow/storage.rspackages/tokens/src/rwa/compliance/modules/country_restrict/mod.rspackages/tokens/src/rwa/compliance/modules/country_restrict/storage.rspackages/tokens/src/rwa/compliance/modules/initial_lockup_period/mod.rspackages/tokens/src/rwa/compliance/modules/initial_lockup_period/storage.rspackages/tokens/src/rwa/compliance/modules/initial_lockup_period/test.rspackages/tokens/src/rwa/compliance/modules/max_balance/mod.rspackages/tokens/src/rwa/compliance/modules/max_balance/storage.rspackages/tokens/src/rwa/compliance/modules/max_balance/test.rspackages/tokens/src/rwa/compliance/modules/mod.rspackages/tokens/src/rwa/compliance/modules/supply_limit/mod.rspackages/tokens/src/rwa/compliance/modules/supply_limit/storage.rspackages/tokens/src/rwa/compliance/modules/supply_limit/test.rspackages/tokens/src/rwa/compliance/modules/time_transfers_limits/mod.rspackages/tokens/src/rwa/compliance/modules/time_transfers_limits/storage.rspackages/tokens/src/rwa/compliance/modules/time_transfers_limits/test.rspackages/tokens/src/rwa/compliance/modules/transfer_restrict/mod.rspackages/tokens/src/rwa/compliance/modules/transfer_restrict/storage.rspackages/tokens/src/rwa/compliance/modules/transfer_restrict/test.rs
Comment on lines
+47
to
+55
| for pkg in "${ALL[@]}"; do | ||
| WASM_NAME="${pkg//-/_}.wasm" | ||
| if [ -f "$WASM_DIR/$WASM_NAME" ]; then | ||
| SIZE=$(wc -c < "$WASM_DIR/$WASM_NAME" | tr -d ' ') | ||
| echo " $WASM_NAME (${SIZE} bytes)" | ||
| else | ||
| echo " WARNING: $WASM_NAME not found!" | ||
| fi | ||
| done |
Contributor
There was a problem hiding this comment.
Fail the script when expected WASM artifacts are missing.
On Line 53, missing outputs are logged as warnings, but the script still exits successfully. That can mask broken builds and cause harder-to-debug downstream deploy failures.
Suggested fix
echo ""
echo "=== WASM sizes ==="
+missing=0
for pkg in "${ALL[@]}"; do
WASM_NAME="${pkg//-/_}.wasm"
if [ -f "$WASM_DIR/$WASM_NAME" ]; then
SIZE=$(wc -c < "$WASM_DIR/$WASM_NAME" | tr -d ' ')
echo " $WASM_NAME (${SIZE} bytes)"
else
echo " WARNING: $WASM_NAME not found!"
+ missing=1
fi
done
+if [ "$missing" -ne 0 ]; then
+ echo "ERROR: One or more expected WASM artifacts are missing." >&2
+ exit 1
+fi
+
echo ""
echo "=== All WASMs built to examples/rwa-deploy/wasm/ ==="🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/rwa-deploy/scripts/build.sh` around lines 47 - 55, The loop that
iterates over ALL and prints warnings when WASM files under WASM_DIR are missing
should instead cause the script to fail; modify the for-loop that sets WASM_NAME
and checks -f to record missing artifacts (e.g., set a flag or increment a
counter when the else branch runs) and after the loop exit with a non-zero
status (exit 1) if any missing were detected; update the echo in the else branch
to remain informative but ensure the script returns failure by using the
recorded flag/counter rather than leaving the current warning-only behavior.
Comment on lines
+62
to
+65
| fn set_identity_registry_storage(e: &Env, token: Address, irs: Address) { | ||
| get_compliance_address(e).require_auth(); | ||
| set_irs_address(e, &token, &irs); | ||
| } |
Contributor
There was a problem hiding this comment.
Freeze the IRS binding once balances exist.
IDBalance is persisted by identity, so swapping the registry contract after pre_set_module_state or any hook update changes the key space without migrating those rows. Existing holders can then look like they have a zero tracked balance under the new registry, which lets incoming transfers/mints bypass the cap until state is rebuilt and can make later debits underflow. Please reject registry changes once balances have been seeded. The standalone example contract mirrors this setter at Lines 53-56 in examples/rwa-max-balance/src/lib.rs, so it needs the same guard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/tokens/src/rwa/compliance/modules/max_balance/mod.rs` around lines
62 - 65, The setter set_identity_registry_storage should reject changing the IRS
once per-identity balances exist: before calling set_irs_address, check the
IDBalance storage (e.g., iterate or query IDBalance entries) and revert if any
non-zero/any rows exist (so registry swaps are blocked after balances are
seeded); keep the existing require_auth check. Also add the identical guard to
the example contract's equivalent setter used in pre_set_module_state so both
places prevent registry swaps after balances exist.
Comment on lines
+119
to
+122
| fn set_identity_registry_storage(e: &Env, token: Address, irs: Address) { | ||
| get_compliance_address(e).require_auth(); | ||
| set_irs_address(e, &token, &irs); | ||
| } |
Contributor
There was a problem hiding this comment.
Freeze the IRS binding once counters have started accruing.
get_counter keys state by (token, identity, limit_time), so changing the registry contract after counters exist makes all active windows invisible under the new identity mapping. The sender effectively starts from zero again, bypassing the transfer-volume limits until every counter is reseeded. Please make this binding one-way or reject changes after the module has been initialized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/tokens/src/rwa/compliance/modules/time_transfers_limits/mod.rs`
around lines 119 - 122, The set_identity_registry_storage function currently
allows swapping the IRS after counters exist; change it to make the IRS binding
immutable once counters have begun accruing by rejecting updates when any
counter exists for that token. In set_identity_registry_storage, before calling
set_irs_address, check the counters storage (via get_counter or the underlying
counters map) for the given token to detect any non-zero/initialized window
entries (or alternatively add and check an initialized flag per token when the
first counter is created); if any counters are present, return/reject the change
(require_auth remains) and only allow set_irs_address when no counters exist
yet. Ensure you reference get_counter, set_identity_registry_storage,
set_irs_address and get_compliance_address when making the change.
Comment on lines
+158
to
+177
| fn remove_time_transfer_limit(e: &Env, token: Address, limit_time: u64) { | ||
| get_compliance_address(e).require_auth(); | ||
| let mut limits = get_limits(e, &token); | ||
|
|
||
| let mut found = false; | ||
| for i in 0..limits.len() { | ||
| let current = limits.get(i).expect("limit exists"); | ||
| if current.limit_time == limit_time { | ||
| limits.remove(i); | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if !found { | ||
| panic_with_error!(e, ComplianceModuleError::MissingLimit); | ||
| } | ||
|
|
||
| set_limits(e, &token, &limits); | ||
| TimeTransferLimitRemoved { token, limit_time }.publish(e); |
Contributor
There was a problem hiding this comment.
Removing a limit leaves stale counters behind.
remove_time_transfer_limit drops the config but keeps every (token, identity, limit_time) counter in storage. If the same limit_time is added again before those timers expire, can_transfer resumes from pre-removal volume while ignoring transfers made during the disabled interval, so enforcement becomes inconsistent across config changes. Consider versioning the counter key or bumping an epoch whenever a window is removed/recreated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/tokens/src/rwa/compliance/modules/time_transfers_limits/mod.rs`
around lines 158 - 177, The removal logic in remove_time_transfer_limit leaves
per-(token,identity,limit_time) counters in storage, allowing old counters to
incorrectly influence future windows; update remove_time_transfer_limit to also
invalidate or version associated counters when a limit is removed (or increment
a window/epoch stored with the limit). Specifically, when removing the limit
from get_limits / set_limits, either (a) delete or mark expired all counters
keyed by (token, limit_time) (and identity) or (b) add/modify a version/epoch
field on the TimeTransferLimit struct and persist that change so can_transfer
(and the counter key builder) uses the version/epoch when reading/updating
counters; change the counter key construction used in can_transfer to include
this version/epoch so re-adding a limit starts fresh.
Keep the retrying invoke helper in common.sh, remove script-local duplicates, and use a neutral retry message across the deploy scripts.
Contributor
Author
|
Marking this draft for now. This PR is intentionally self-contained against current |
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
1 participant
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.
Summary
Adds deployable RWA example contracts together with scripts for building, deploying, wiring, and running testnet end-to-end validation.
Changes
examples/rwa-deploy/: adds deploy crates for compliance, token, IRS, and verifier, plus the deployment README.build.sh,build-module.sh,deploy.sh,deploy-module.sh,wire.sh,e2e.sh, andtest-happy-path.sh.main, including the IRS CLI payload encoding expected by the current contract interface.Test plan
cargo +nightly fmt --all -- --checkcargo test -p deploy-irs --libcargo test -p deploy-verifier --libcargo test -p deploy-compliance --libcargo test -p deploy-token --libPATH="/usr/local/bin:$PATH" ./examples/rwa-deploy/scripts/e2e.sh --skip-build