fix: harden adapter accounting and Blend admin boundary#431
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 refactors vault validation logic across kernel and Soroban layers. It removes the kernel's conservative "would more than double total assets" check and replaces it with operation-state-gated validation. The Blend adapter constructor now validates that admin, vault, and pool are contract addresses. Vault allocations and refreshes now stage principal updates and validate bounds before committing. Token balance verification in withdrawals ensures reported deltas match actual balance changes. Comprehensive tests validate both kernel behavior and end-to-end adapter interactions. ChangesVault validation and asset sync refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
e951f6f to
10806a4
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/kernel/tests/property_tests.rs`:
- Around line 3818-3824: The property test currently limits delta by calling
delta.min(in_flight), which re-introduces the old in-flight cap and prevents
exercising larger valid sync updates; remove that cap by not clamping delta to
in_flight (i.e., stop using delta.min(in_flight)) so delta can exceed in_flight
in generated cases and compute external = existing_external + delta as intended;
update the generator variables (existing_external, in_flight, delta) and
subsequent assertions in the test around VaultState::new and any uses of
in_flight to ensure the test logic expects delta > in_flight scenarios.
In `@contract/vault/soroban/blend-adapter/src/lib.rs`:
- Around line 55-64: The constructor currently validates addresses using
string-prefix-based is_contract_address; replace that with payload-based checks
by calling AddressPayload::from_address(&address) and pattern-matching for
AddressPayload::ContractIdHash to determine contract addresses, and return
AdapterError::InvalidInput via require_contract_address (or update that helper
to use AddressPayload::from_address) for admin, vault, and pool in __constructor
so validation relies on AddressPayload rather than to_string()/byte-prefix
inspection.
🪄 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: cab4b0d8-cf13-4999-adc6-144c884042d1
📒 Files selected for processing (11)
contract/vault/kernel/src/actions/mod.rscontract/vault/kernel/src/actions/tests.rscontract/vault/kernel/src/error.rscontract/vault/kernel/tests/property_tests.rscontract/vault/soroban/blend-adapter/src/lib.rscontract/vault/soroban/blend-adapter/tests/integration_tests.rscontract/vault/soroban/justfilecontract/vault/soroban/src/contract/curator_vault.rscontract/vault/soroban/src/contract/entrypoints.rscontract/vault/soroban/src/tests.rscontract/vault/soroban/tests/blend_e2e.rs
💤 Files with no reviewable changes (3)
- contract/vault/soroban/justfile
- contract/vault/kernel/src/actions/mod.rs
- contract/vault/kernel/src/error.rs
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…f-9a55-4006-b3fc-e5af3fbb7605) Trace: 3a4a7ae
…a9-4b9a-867a-228152a95c3b) Trace: 80836ac
…18d-4fd1-bff3-109530b96144) Trace: e1d1ce9
…460a-b176-bc982fb254cd) Trace: 23878c9
…5a3ec7-5477-4c2f-9488-30308ab956d6) Trace: e951f6f
…3ec7-5477-4c2f-9488-30308ab956d6) Use Soroban AddressPayload for Blend adapter contract-address discrimination instead of parsing address strings, and keep the external-asset sync property from clamping generated deltas to in-flight allocation amounts. Verification: - cargo fmt --check - cargo test -p templar-soroban-blend-adapter -- --nocapture - cargo test -p templar-vault-kernel --test property_tests --features action-sync-external prop_spec_sync_external_assets_updates_total -- --nocapture
da34b37 to
4193c59
Compare
Summary
Consolidated adapter follow-up PR for the tracker-defined
adapter-accounting-and-blend-adminboundary. This supersedes the previously split draft PRs #433, #434, #438, and #439.Findings covered:
0c991b9f-9a55-4006-b3fc-e5af3fbb7605bc013b77-b8a9-4b9a-867a-228152a95c3b76f200c7-618d-4fd1-bff3-109530b96144supply_balance,withdraw_to_vault) so managed positions mutate only through the vault-routed lifecycle.81068879-6d1a-460a-b176-bc982fb254cdset_pool,set_vault) and stale retarget events/recipes.6a5a3ec7-5477-4c2f-9488-30308ab956d6Tracker / PR boundary note
The audit tracker's
adapter-identity-and-governancecluster calls for a single follow-up PR boundary namedadapter-accounting-and-blend-adminfor A-032/A-033/A-034/A-035 plus A-060 if accepted. This PR is now that consolidated boundary.Superseded split PRs:
Verification
cargo fmt --all -- --checkgit diff --checkset_pool,set_vault,supply_balance,withdraw_to_vault,pool_upd, orvlt_updsymbolsCARGO_INCREMENTAL=0 CARGO_TARGET_DIR=/data/tmp/contracts-adapter-consolidated/.target-consolidated cargo test -p templar-soroban-blend-adapter -- --nocapture— 16 unit + 7 integration passedCARGO_INCREMENTAL=0 CARGO_TARGET_DIR=/data/tmp/contracts-adapter-consolidated/.target-consolidated cargo test -p templar-soroban-runtime --features testutils complete_refresh -- --nocapture— 3 runtime tests + 1 property-filtered test passedCARGO_INCREMENTAL=0 CARGO_TARGET_DIR=/data/tmp/contracts-adapter-consolidated/.target-consolidated cargo test -p templar-soroban-runtime --test blend_e2e withdraw_allocation_rejects_adapter_reported_assets_without_matching_balance_delta -- --nocapture— 1 passedCARGO_INCREMENTAL=0 CARGO_TARGET_DIR=/data/tmp/contracts-adapter-consolidated/.target-consolidated cargo test -p templar-vault-kernel sync_external_assets -- --nocapture— 10 unit + 1 property-filtered test passedsize-budget-checkhook passed on consolidated commits at95908bytesThis change is