Harden share-token TTL and archival runbook#445
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces time-to-live (TTL) management for the Soroban share-token contract. Read-only token methods now extend the contract instance TTL before returning values. An admin-only ChangesShare-Token TTL Extension and Allowance Expiry
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 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)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
carrion256
force-pushed
the
audit/share-token-admin-immutable
branch
from
3c5d098 to
f8dedf6
Compare
carrion256
force-pushed
the
audit/share-token-ttl-and-archival
branch
from
e189e24 to
396f310
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 `@contract/vault/soroban/share-token/src/tests.rs`:
- Around line 470-525: The test
read_only_entrypoints_cover_share_token_ttl_maintenance_surface currently only
asserts token values and must also verify TTL movement: read and store the
contract instance TTL for token before calling the read-only entrypoint(s) (use
env to query the instance TTL), invoke the read-only extend_instance_ttl
entrypoint via env.invoke_contract (Symbol::new(&env, "extend_instance_ttl") or
the specific read-only method you want to exercise), then read the instance TTL
again and assert it has advanced/been refreshed; update the test to perform this
before/after TTL assertion using the existing token variable and the test
function read_only_entrypoints_cover_share_token_ttl_maintenance_surface.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 44083f83-a1fa-4e04-b033-e728e8dbc679
📒 Files selected for processing (3)
contract/vault/soroban/README.mdcontract/vault/soroban/share-token/src/lib.rscontract/vault/soroban/share-token/src/tests.rs
| #[test] | ||
| fn read_only_entrypoints_cover_share_token_ttl_maintenance_surface() { | ||
| let (env, _admin, vault, token) = setup(); | ||
| let user = Address::generate(&env); | ||
| let spender = Address::generate(&env); | ||
|
|
||
| env.as_contract(&vault, || { | ||
| VaultCaller::mint(env.clone(), token.clone(), user.clone(), 1000); | ||
| VaultCaller::approve( | ||
| env.clone(), | ||
| token.clone(), | ||
| user.clone(), | ||
| spender.clone(), | ||
| 250, | ||
| 300, | ||
| ); | ||
| }); | ||
|
|
||
| let supply: i128 = env.invoke_contract( | ||
| &token, | ||
| &soroban_sdk::Symbol::new(&env, "total_supply"), | ||
| ().into_val(&env), | ||
| ); | ||
| let balance: i128 = env.invoke_contract( | ||
| &token, | ||
| &soroban_sdk::Symbol::new(&env, "balance"), | ||
| (&user,).into_val(&env), | ||
| ); | ||
| let allowance: i128 = env.invoke_contract( | ||
| &token, | ||
| &soroban_sdk::Symbol::new(&env, "allowance"), | ||
| (&user, &spender).into_val(&env), | ||
| ); | ||
| let name: String = env.invoke_contract( | ||
| &token, | ||
| &soroban_sdk::Symbol::new(&env, "name"), | ||
| ().into_val(&env), | ||
| ); | ||
| let symbol: String = env.invoke_contract( | ||
| &token, | ||
| &soroban_sdk::Symbol::new(&env, "symbol"), | ||
| ().into_val(&env), | ||
| ); | ||
| let decimals: u32 = env.invoke_contract( | ||
| &token, | ||
| &soroban_sdk::Symbol::new(&env, "decimals"), | ||
| ().into_val(&env), | ||
| ); | ||
|
|
||
| assert_eq!(supply, 1000); | ||
| assert_eq!(balance, 1000); | ||
| assert_eq!(allowance, 250); | ||
| assert_eq!(name, String::from_str(&env, "Templar Share")); | ||
| assert_eq!(symbol, String::from_str(&env, "tvSHARE")); | ||
| assert_eq!(decimals, 7); | ||
| } |
There was a problem hiding this comment.
TTL regression test is value-only and does not prove TTL refresh.
On Line 471, this test name states TTL maintenance coverage, but it only asserts token values. It would still pass even if the read-only extend_instance_ttl calls were removed. Please add an assertion that verifies observable instance-TTL movement before/after at least one of these read-only calls.
🤖 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 `@contract/vault/soroban/share-token/src/tests.rs` around lines 470 - 525, The
test read_only_entrypoints_cover_share_token_ttl_maintenance_surface currently
only asserts token values and must also verify TTL movement: read and store the
contract instance TTL for token before calling the read-only entrypoint(s) (use
env to query the instance TTL), invoke the read-only extend_instance_ttl
entrypoint via env.invoke_contract (Symbol::new(&env, "extend_instance_ttl") or
the specific read-only method you want to exercise), then read the instance TTL
again and assert it has advanced/been refreshed; update the test to perform this
before/after TTL assertion using the existing token variable and the test
function read_only_entrypoints_cover_share_token_ttl_maintenance_surface.
467fdf8 to
8470b7f
Compare
…xus 1423b989-f1cf-4827-bc37-6d76d00018f8 Nexus 8e84564c-0b32-40b9-8bc8-227b2ca23395 Nexus 1122e421-af18-43d0-9062-bb44dbe948cc) Trace: e189e24
carrion256
force-pushed
the
audit/share-token-ttl-and-archival
branch
from
396f310 to
8b96576
Compare
…953-4ed7-9971-7e335029ad0a)
1 participant
Summary
Fixes/triages the share-token TTL and archival follow-up findings:
1423b989-f1cf-4827-bc37-6d76d00018f8— document the Soroban archival restore story for the share token.8e84564c-0b32-40b9-8bc8-227b2ca23395— refresh instance TTL from all SEP-41 read-only methods.1122e421-af18-43d0-9062-bb44dbe948cc— document/prove why adminextend_ttlremains instance-only while holder balances/allowances keep per-entry semantics.This is stacked on the consolidated share-token PR branch
audit/share-token-admin-immutable.Changes
extend_instance_ttl(e)to SEP-41 read-only methods:total_supply,balance,allowance,decimals,name,symbol.extend_ttlfrom the configured admin.stellar-tokensrefreshes touched persistent holder balances on balance reads/writes;live_until_ledger.extend_ttlbalance/allowance semantics.Verification
RUSTUP_TOOLCHAIN=1.89.0 cargo fmt --all --checkRUSTUP_TOOLCHAIN=1.89.0 CARGO_TARGET_DIR=/data/tmp/contracts-share-token-ttl-archival/target cargo test -p templar-soroban-share-token read_only_entrypoints_cover_share_token_ttl_maintenance_surface -- --nocaptureRUSTUP_TOOLCHAIN=1.89.0 CARGO_TARGET_DIR=/data/tmp/contracts-share-token-ttl-archival/target cargo test -p templar-soroban-share-token admin_extend_ttl_preserves_holder_balances_and_allowance_expiry_semantics -- --nocaptureRUSTUP_TOOLCHAIN=1.89.0 CARGO_TARGET_DIR=/data/tmp/contracts-share-token-ttl-archival/target cargo test -p templar-soroban-share-token -- --nocapture— 22 passedgit diff --checksize-budget-check— passed at93961bytesTracker
Updated local tracker rows and cluster notes:
contracts-audit-findings-index.mdcontracts-audit-clusters/storage-versioning-ttl.audit.mdcontracts-audit-clusters/share-token-admin.audit.mdThis change is