Conversation
Transplant the reviewed country compliance modules and example crates onto upstream/main so they can ship as an independent PR without the old stack.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces two new RWA compliance modules— Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/tokens/src/rwa/compliance/modules/country_restrict/storage.rs (1)
19-28: Optional: Consider extracting a shared storage helper.The
country_allowandcountry_restrictstorage modules share nearly identical logic (read with TTL extension, set with TTL, remove). If more country-based modules are anticipated, a generic helper could reduce duplication. However, keeping them separate is reasonable for clarity and independent evolution.Also applies to: 37-41, 50-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tokens/src/rwa/compliance/modules/country_restrict/storage.rs` around lines 19 - 28, Extract the repeated TTL-aware storage operations into a shared helper to avoid duplication across the country_allow and country_restrict modules: create a small utility (e.g., functions like get_with_ttl_extend, set_with_ttl, remove_with_ttl) that accepts the Env reference, a storage key type (used by CountryRestrictStorageKey and the country_allow key), TTL threshold and extend amounts, and a closure/type param for the value; then replace the body of is_country_restricted (and the analogous functions at the other locations) to call get_with_ttl_extend instead of duplicating e.storage().persistent().get(...).inspect(...).unwrap_or_default(), and use set_with_ttl/remove_with_ttl for writes and deletes so all TTL logic is centralized and consistent.examples/rwa-country-restrict/src/lib.rs (1)
3-3: Remove unusedStringimport.The
Stringtype is imported but never used in this file.Suggested fix
-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-restrict/src/lib.rs` at line 3, The import list from soroban_sdk includes an unused symbol `String`; remove `String` from the `use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String, Vec};` statement so the import becomes only the actually used symbols (`contract`, `contractimpl`, `contracttype`, `Address`, `Env`, `Vec`), eliminating the unused `String` import and any related compiler warnings.examples/rwa-country-allow/src/lib.rs (1)
3-3: Remove unusedStringimport.The
Stringtype is imported but never used in this file.Suggested fix
-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, Remove the unused String import from the module-level use statement: in the use declaration that currently lists contract, contractimpl, contracttype, Address, Env, String, Vec remove the String identifier so only used symbols remain; ensure no other code references String in this file before committing the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/rwa-country-allow/src/lib.rs`:
- Line 3: Remove the unused String import from the module-level use statement:
in the use declaration that currently lists contract, contractimpl,
contracttype, Address, Env, String, Vec remove the String identifier so only
used symbols remain; ensure no other code references String in this file before
committing the change.
In `@examples/rwa-country-restrict/src/lib.rs`:
- Line 3: The import list from soroban_sdk includes an unused symbol `String`;
remove `String` from the `use soroban_sdk::{contract, contractimpl,
contracttype, Address, Env, String, Vec};` statement so the import becomes only
the actually used symbols (`contract`, `contractimpl`, `contracttype`,
`Address`, `Env`, `Vec`), eliminating the unused `String` import and any related
compiler warnings.
In `@packages/tokens/src/rwa/compliance/modules/country_restrict/storage.rs`:
- Around line 19-28: Extract the repeated TTL-aware storage operations into a
shared helper to avoid duplication across the country_allow and country_restrict
modules: create a small utility (e.g., functions like get_with_ttl_extend,
set_with_ttl, remove_with_ttl) that accepts the Env reference, a storage key
type (used by CountryRestrictStorageKey and the country_allow key), TTL
threshold and extend amounts, and a closure/type param for the value; then
replace the body of is_country_restricted (and the analogous functions at the
other locations) to call get_with_ttl_extend instead of duplicating
e.storage().persistent().get(...).inspect(...).unwrap_or_default(), and use
set_with_ttl/remove_with_ttl for writes and deletes so all TTL logic is
centralized and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a63ad065-ba2e-4f2b-b4b6-cecc0c0c99a3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
Cargo.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.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/mod.rs
Align the country compliance modules with the repository's preferred test layout by extracting inline test modules into dedicated test.rs files.
| } | ||
|
|
||
| #[contractimpl(contracttrait)] | ||
| impl CountryAllow for CountryAllowContract { |
There was a problem hiding this comment.
I think all the implementations below can be skipped with default implementations. The only problem may be require_module_admin_or_compliance_auth, but I think we can workaround that. In fact, that limitation itself requires a bit discussion imo
ozgunozerk
left a comment
There was a problem hiding this comment.
examples are not done in the light of our other examples. The structure is missing, there is only lib file, and an unwanted readme.
There isn't contract, nor tests.
Move country_allow and country_restrict module logic into public free functions in storage.rs. Remove per-module contracttrait definitions from mod.rs. Restructure examples into canonical lib.rs / contract.rs layout. Delete example READMEs.
Thanks, I updated this to be closer to the structure of the other examples. The example now has the usual split, the extra README is gone, and I added local tests for both country examples so they’re easier to review and a bit more complete. I left the repeated methods as-is for now instead of trying to refactor that part in this PR. That felt like a bigger cleanup than I wanted to mix into what was meant to stay a structural-only pass. If you want, I can take that on next in a separate follow-up. |
Align the country standalone examples and module docs with the repo's compliance-module conventions. Add example-local coverage for the admin/compliance auth boundary so the reviewed structure is verified without changing module behavior.
Country Compliance Modules
This PR introduces two country-based compliance modules for RWA token management:
Country Allow
Only recipients whose identity has at least one country code present in the allowlist may receive tokens. Administrators can configure which countries are allowed per token, either individually or in batch.
Country Restrict
Recipients whose identity has a country code on the restriction list are blocked from receiving tokens. Administrators can configure which countries are restricted per token, either individually or in batch.