Skip to content

[codex] fix proxy curator governance facade routes#464

Merged
carrion256 merged 1 commit into
devfrom
fix/proxy-curator-facade-entrypoints
Jun 8, 2026
Merged

[codex] fix proxy curator governance facade routes#464
carrion256 merged 1 commit into
devfrom
fix/proxy-curator-facade-entrypoints

Conversation

@carrion256

@carrion256 carrion256 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • fix proxy-curator-soroban set_supply_queue to forward SupplyQueueProposalEntry values with target ids and adapter addresses
  • route cancel_migration through governance submit_cancel_migration instead of direct vault execution
  • add regression tests for both facade routes and ignore generated proxy Soroban test snapshots

Root cause

The new curator proxy facade exposed helper signatures that did not match the underlying governance/vault contracts. set_supply_queue forwarded only market ids, and cancel_migration bypassed the governance contract even though the vault only accepts that command from governance.

Validation

  • cargo test -p templar-curator-proxy-soroban -- --nocapture
  • post-commit Soroban size-budget-check passed: 123720 bytes

This change is Reviewable

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary

This PR fixes governance routing in the proxy-curator-soroban contract's facade for two critical operations: cancel_migration and set_supply_queue. Both methods previously had signature or routing mismatches with the underlying governance contract.

Key Changes

contract/proxy-curator-soroban/src/contract.rs:

  • Removed CancelMigration variant from VaultCommand enum; cancel_migration() now submits a governance action via invoke_governance("submit_cancel_migration", ...) instead of executing directly on the vault
  • Changed cancel_migration() return type from Result<(), ContractError> to Result<u64, ContractError> (returns proposal ID)
  • Changed set_supply_queue() parameter from markets: Vec<u32> to entries: Vec<SupplyQueueProposalEntry> and now routes through governance via "submit_set_supply_queue"
  • Both methods already enforce authorization via admin.require_auth()

contract/proxy-curator-soroban/src/lib.rs:

  • Exported SupplyQueueProposalEntry type from templar_soroban_governance

contract/proxy-curator-soroban/src/tests.rs:

  • Extended mock governance contract with submit_set_supply_queue() and submit_cancel_migration() stub methods
  • Added cancel_migration_submits_timelocked_governance_proposal test to validate governance submission
  • Extended governance facade test to cover set_supply_queue with full SupplyQueueProposalEntry objects
  • Updated vault mock to remove assertions on CancelMigration payloads

.gitignore:

  • Added proxy Soroban test snapshot directories to ignore patterns

Critical Review Points

  1. Governance enforcement correctness: Both operations now route through the governance contract, preventing bypassing of approval workflows. Verify that the underlying governance contract's submit_cancel_migration and submit_set_supply_queue accept the exact parameter signatures being passed.

  2. Payload completeness: The set_supply_queue now forwards full SupplyQueueProposalEntry structures (including target IDs and adapter addresses). Confirm these fields are correctly populated by callers and accepted by the governance contract.

  3. Return type change: cancel_migration now returns u64 instead of (). Verify all callers can handle the proposal ID return and don't expect a void return.

  4. Authorization boundary: Both operations require admin.require_auth(). Confirm that the governance contract properly validates the caller's identity on the submission side.

  5. Backwards compatibility: The vault command enum no longer includes CancelMigration. Verify no other code paths attempt to construct or match this variant.

Walkthrough

The PR migrates the curator proxy's cancel_migration and set_supply_queue operations from direct vault-command execution to governance action submission, updating method signatures and test coverage to reflect this semantic shift.

Changes

Curator Proxy Governance Submission Semantics

Layer / File(s) Summary
Type imports and re-exports
contract/proxy-curator-soroban/src/contract.rs, contract/proxy-curator-soroban/src/lib.rs
SupplyQueueProposalEntry is imported from governance types and re-exported from the proxy contract's public API.
Vault command removal
contract/proxy-curator-soroban/src/contract.rs
The VaultCommand enum no longer includes CancelMigration, and its serialization mapping is removed from into_wire().
Cancel migration and supply-queue governance submission
contract/proxy-curator-soroban/src/contract.rs
cancel_migration now returns u64 and submits a submit_cancel_migration governance action; set_supply_queue accepts Vec<SupplyQueueProposalEntry> and submits entries via submit_set_supply_queue instead of vault commands.
Mock governance infrastructure
contract/proxy-curator-soroban/src/tests.rs
Mock governance is extended with storage keys and typed call structs (MockSetSupplyQueueCall, MockCancelMigrationCall) and corresponding submit/retrieve methods to record and retrieve typed governance submissions.
Test assertions and coverage
contract/proxy-curator-soroban/src/tests.rs
Vault-operations test expectations are adjusted to remove cancel-migration assertions; a new test validates cancel-migration governance submission returns the expected proposal ID and records no vault payloads; the typed-governance test is extended to verify set-supply-queue forwarding with entries matching the input.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • peer2f00l
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/proxy-curator-facade-entrypoints

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • LINEAR integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@carrion256
carrion256 marked this pull request as ready for review June 8, 2026 13:34
@carrion256
carrion256 enabled auto-merge (rebase) June 8, 2026 13:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.

Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.

👉 Steps to fix this

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/proxy-curator-soroban/src/tests.rs`:
- Around line 623-649: The test only verifies a single SupplyQueueProposalEntry
and can miss truncation/reordering bugs; update the test that calls
SorobanCuratorProxyContract::set_supply_queue to use at least two
SupplyQueueProposalEntry elements when constructing supply_entries (the
Vec::from_array call), then after retrieving supply_queue_call via
MockGovernanceContract::last_set_supply_queue assert entries.len() == 2 and
assert the returned entries vector is equal to the original supply_entries (or
perform element-wise equality checks using entries.get_unchecked(i) ==
supply_entries.get_unchecked(i) for both indices) to ensure full-vector equality
and order is preserved.
🪄 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: 10c6e3c6-0cf1-412e-afce-1bd54d106d67

📥 Commits

Reviewing files that changed from the base of the PR and between 72c8a43 and d0ba01d.

📒 Files selected for processing (4)
  • .gitignore
  • contract/proxy-curator-soroban/src/contract.rs
  • contract/proxy-curator-soroban/src/lib.rs
  • contract/proxy-curator-soroban/src/tests.rs

Comment thread contract/proxy-curator-soroban/src/tests.rs
@carrion256
carrion256 merged commit 0c64209 into dev Jun 8, 2026
26 checks passed
@carrion256
carrion256 deleted the fix/proxy-curator-facade-entrypoints branch June 8, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant