Skip to content

Fix: 4 issues (#226, #238, #239, #242) - #301

Merged
techisigu merged 1 commit into
Zenith-options:mainfrom
chideraisiguzor:simple/226-238-239-242
Oct 24, 2026
Merged

techisigu merged 1 commit into
Zenith-options:mainfrom
chideraisiguzor:simple/226-238-239-242

Conversation

@chideraisiguzor

Copy link
Copy Markdown
Contributor

Closes #226
Closes #238
Closes #239
Closes #242

Issues

#226 Enforce Workspace-Wide cargo fmt and Clippy in CI

Description:
Add CI jobs that enforce cargo fmt (using the existing rustfmt.toml) and cargo clippy -- -D warnings across every workspace member, so the crate-by-crate reconstruction lands on a lint-clean, uniformly formatted baseline rather than accumulating drift.

Problem Statement & Context:
The repo already carries rustfmt.toml and .cargo/ configuration, but nothing under .github/ enforces either one. While four contracts are being reconstructed in parallel and stub-heavy crates are still being filled in, unreviewed formatting churn inflates diffs on files like options_market/src/lib.rs, and genuine clippy findings get buried under a wall of pre-existing warnings.

Scope & Acceptance Criteria:

  • Add a lint job under .github/workflows/ running cargo fmt --all -- --check and cargo clippy --workspace --all-targets -- -D warnings.
  • The job runs on every pull request and on main, and failures block merge.
  • Any pre-existing lint that cannot be fixed immediately is captured as a narrowly scoped, commented #[allow(...)] with a tracked follow-up, never a blanket workspace-wide allow.
  • Out of scope: the workspace compile/stub-detection gate, coverage thresholds, wasm builds/size budgets, and static security analysis — each is tracked by its own CI issue.
  • Because clippy requires compilable code, the job may land advisory-only, but the flap that switches it to blocking must be explicit and documented in the workflow file.

Implementation Guidelines:

  1. Key Files/Modules: .github/workflows/, rustfmt.toml, .cargo/, workspace Cargo.toml.
  2. Design/Architecture: one job with a cached toolchain; run fmt and clippy as distinct steps so failures are attributable; pin the toolchain to the same version used by the other CI workflows.
  3. Edge Cases/Constraints: contract crates are wasm-oriented and #[contractimpl] expands macro-generated code that can trip lints — prefer item-level allows over crate-level ones; coordinate landing order with the compile gate so the job is not permanently red.
  4. Testing: prove the job fails by pushing a scratch branch containing one unformatted line and one deliberate warning, then confirm it passes on the cleaned workspace.

Definition of "Done":

  • Workflow committed, tested, and documented.
  • All acceptance criteria met.
  • PR submitted with a clear description, passing CI, and sample failure/pass output attached.
  • Code reviewed and approved.

Resources: Related CI issues #‍124, #‍125, #‍126, #‍127, #‍128.

Complexity: Medium (40 points)

#238 Implement the params Crate: On-Chain Protocol Parameter Registry

Description:
Implement params/ as a working Soroban contract that holds protocol-wide configuration (oracle staleness windows, quorum sizes, fee bps, timelock delays) under namespaced typed keys, so parameters can be read and updated in one governed place instead of being hard-coded constants scattered across crates.

Problem Statement & Context:
Each reconstructed contract defines its own limits and defaults, and there is no single place to answer "what is the current minimum feeder quorum?" or to change it without a contract-specific admin call and a redeploy. A typed registry makes governance of parameters auditable and lets timelock/multisig execute parameter changes with the same delay as any other admin action.

Scope & Acceptance Criteria:

  • Implement set_value, get_value, remove_value, and list_namespace with support for at least u32, i64, bool, and Address value types.
  • Keys are namespaced per consuming contract (e.g. (price_oracle, "min_reports")); a caller can never read or write another namespace's key without passing the namespace check.
  • Writes gated through common::require_admin_or_multisig; reads are permissionless.
  • Getters support an explicit default so a consuming contract can fall back when a parameter is unset.
  • Emit an event per set/remove carrying the namespace, key, and old/new value.
  • Out of scope: retrofitting every existing constant in options_market, vault, and price_oracle to read from params (track that as follow-up work per contract).

Implementation Guidelines:

  1. Key Files/Modules: params/ (contract, types, storage, events, error), common/ (auth + pause helpers), timelock/ and multisig/ (the intended writers), integration_tests/.
  2. Design/Architecture: encode key and type tag into a single storage key so a u32 and an i64 under the same name cannot collide; store values in a small enum rather than serializing untyped bytes.
  3. Edge Cases/Constraints: type mismatch on read (fail closed with a specific error code, never silently coerce), removing an unset key, enumerating a namespace with many entries (page the listing), and TTL extension on long-lived parameter entries.
  4. Testing: unit tests per value type for set/get/remove round-trips, namespace isolation, type-mismatch rejection, default fallback behaviour, and a negative test that a non-admin cannot write.

Definition of "Done":

  • Code written, tested, and documented.
  • All acceptance criteria met.
  • PR submitted with a clear description, passing CI, relevant screenshots/test output.
  • Code reviewed and approved.

Complexity: Medium (100 points)

#239 Implement lp_pool Epoch Deposit/Withdrawal Queues and Share-Price Accounting

Description:
Implement lp_pool/ as a working Soroban contract: epoch-queued deposits and withdrawals, a share price computed per epoch rather than per transaction, and writers that deploy pooled capital as option collateral on options_market. The README describes this crate and links docs/lp_pool.md, but the entrypoints behind that description are not wired up.

Problem Statement & Context:
Without a functioning LP pool, the protocol only has individual writers locking their own collateral — there is no way for passive capital to underwrite options, which is the main source of depth in the order book. Epoch batching is what makes the share price un-manipulable by same-ledger deposit/withdraw round-trips.

Scope & Acceptance Criteria:

  • Implement request_deposit, request_withdraw, process_epoch (or equivalent finalization), claim, and read-only share_price / pending_* views.
  • Deposits and withdrawals requested during an epoch are only priced at the next finalized epoch; a request cannot be filled at the price it was observed at.
  • Shares are minted/burned through the pinned lp_token; the pool is the only minter.
  • Pool capital used as option collateral goes through options_market's writer flow and is accounted against the corresponding vault tag, so pool funds are never commingled with individual writers' collateral.
  • Withdrawals cannot be filled from collateral that is still locked against a live series.
  • Emit events for deposit request, withdraw request, epoch finalization, and claim.
  • Out of scope: secondary market for shares, per-depositor strategy selection, and any UI.

Implementation Guidelines:

  1. Key Files/Modules: lp_pool/ (contract, types, storage, events, error), lp_token/ (mint/burn), options_market/ and vault/ (collateral deployment and escrow tags), docs/lp_pool.md.
  2. Design/Architecture: one queue per epoch per direction; share price derived from total assets minus locked collateral over total shares at finalization; never compute price from the raw token balance of the pool address.
  3. Edge Cases/Constraints: first epoch with zero shares, an epoch where locked collateral exceeds withdrawable assets, a depositor claiming after several epochs, rounding dust on share burn, and re-requesting in the same epoch.
  4. Testing: unit tests for the queue lifecycle and share-price math including rounding, plus an integration test with options_market and vault showing a pool-written series blocking a withdrawal until settlement.

Definition of "Done":

  • Code written, tested, and documented.
  • All acceptance criteria met.
  • PR submitted with a clear description, passing CI, relevant screenshots/test output.
  • Code reviewed and approved.

Resources:

  • docs/lp_pool.md (design notes for this crate).

Complexity: High (200 points)

#242 Implement streams Stream Recipient Transfer and Assignment

Description:
Add a transfer_recipient entrypoint to the streams contract that reassigns the right to withdraw from an existing stream to a new address, authorized by both the current recipient and the new one. Without this, a stream recipient who wants to hand off (or sell) their position has no path other than waiting for the whole schedule to vest.

Problem Statement & Context:
Vesting positions are economic assets; forcing them to stay with the original address means the only way to realize value early is an off-chain side agreement the contract cannot enforce. A native assignment path keeps the obligation and the entitlement in one place and makes the transfer auditable.

Scope & Acceptance Criteria:

  • transfer_recipient(stream_id, new_recipient) requires authorization from the current recipient and the new recipient.
  • Amounts already withdrawable at the time of transfer stay claimable by the new recipient (the schedule itself is untouched); the previously withdrawable amount is checkpointed in the event for auditability.
  • The stream's sender, token, amount, and schedule are unchanged by a transfer; a sender cannot unilaterally reassign a recipient.
  • Transfers of non-existent or completed streams fail with distinct errors.
  • Emit an event with stream id, old recipient, new recipient, and the withdrawable amount at transfer time.
  • Out of scope: transferring the sender side, transferring a stream to the contract itself, and cancellation/revocation paths.

Implementation Guidelines:

  1. Key Files/Modules: streams/ (contract, storage keyed by recipient, events, error), streams test suite.
  2. Design/Architecture: reindex the stream under the new recipient in the same operation; if the crate keeps a per-recipient list for enumeration, remove from the old list and append to the new one atomically.
  3. Edge Cases/Constraints: transferring to the current recipient (no-op or explicit rejection — pick one and document it), transferring a stream whose schedule is fully vested, and ensuring the old recipient cannot withdraw after the transfer in the same ledger.
  4. Testing: unit tests for the happy path with both auths present, and negative tests where only one of the two signatures is provided, where the stream does not exist, and where the stream is complete.

Definition of "Done":

  • Code written, tested, and documented.
  • All acceptance criteria met.
  • PR submitted with a clear description, passing CI, relevant screenshots/test output.
  • Code reviewed and approved.

Complexity: Low (50 points)

Summary

Land the CI lint job first so subsequent PRs are checked; it may start advisory-only (continue-on-error) and be flipped to blocking once the workspace compiles cleanly, with the flap documented in the workflow file. For params, keep the existing bounds/migration machinery intact and add the namespaced registry alongside it; use item-level #[allow] with tracked follow-ups for any macro-generated lint in contract crates, never crate-level allows. For lp_pool, the existing scaffolding already has most storage/types/events; focus on wiring the entrypoints and share-price math, and ensure the integration test exercises options_market and vault. For streams, the existing transfer entrypoint is recipient-only; add transfer_recipient with dual auth and reindexing, and document the self-transfer decision.

Changes

  • Add a workspace-wide lint job to the CI workflow. Create a lint job (or fix the existing fmt/clippy jobs) that runs cargo fmt --all -- --check and cargo clippy --workspace --all-targets -- -D warnings as two distinct steps, on every pull request and on pushes to main. Pin the toolchain to the same version used by the other jobs (dtolnay/rust-toolchain@stable with rustfmt and clippy components plus wasm32-unknown-unknown target), use Swatinem/rust-cache, and run cargo xtask check-imports before clippy if cross-contract wasm is required. Document in a comment block at the top of the job that it may land advisory-only (continue-on-error: true) while stub-heavy crates are being reconstructed, and that flipping to blocking is done by removing that flag — make the flap explicit and documented. Any pre-existing lint that cannot be fixed must be captured as a narrowly scoped, commented #[allow(...)] with a tracked follow-up, never a workspace-wide allow. Also clean up the duplicated/malformed job blocks currently in the file (the truncated crate: matrix line, the duplicated wasm/test jobs) so the workflow is valid YAML. (.github/workflows/ci.yml)
  • Implement the params crate as a typed, namespaced on-chain parameter registry. Add set_value, get_value, remove_value, and a paginated list_namespace supporting at least u32, i64, bool, and Address values. Encode namespace, key, and a type tag into a single storage key so a u32 and an i64 under the same name cannot collide; store values in a small typed enum rather than untyped bytes. Gate writes through common::require_admin_or_multisig (admin/multisig auth), keep reads permissionless, and support an explicit default on getters so consumers can fall back when a parameter is unset. Fail closed with a specific error code on type mismatch (never silently coerce), handle removing an unset key, page namespace listings, and extend TTL on long-lived parameter entries. Emit an event per set/remove carrying namespace, key, and old/new value. Add unit tests per value type for set/get/remove round-trips, namespace isolation, type-mismatch rejection, default fallback, and a negative test that a non-admin cannot write. (params/src/lib.rs)
  • Extend params/src/types.rs with the typed value enum (ParamValue covering U32(u32), I64(i64), Bool(bool), Address(Address)), a namespaced key type combining namespace Symbol, key Symbol, and a type tag, and the new DataKey variants needed for the registry (namespaced value entries and per-namespace index entries for paginated listing). Keep the existing Param/BoundsProposal types intact so the current bounds machinery still compiles. (params/src/types.rs)
  • Add the new error variants for the namespaced registry: type mismatch on read (e.g. TypeMismatch), key not found where required, invalid namespace/key, and invalid pagination cursor/limit. Keep existing codes stable and append new ones. (params/src/error.rs)
  • Add event helpers for the namespaced registry: a value_set event carrying namespace, key, and old/new value, and a value_removed event carrying namespace, key, and the removed value. Keep existing param/bounds events unchanged. (params/src/events.rs)
  • Add unit tests for the new namespaced registry in params/src/test.rs: set/get/remove round-trips for each value type (u32, i64, bool, Address), namespace isolation (a caller cannot read/write another namespace's key), type-mismatch rejection with the specific error, default fallback when a parameter is unset, paginated list_namespace, and a negative test that a non-admin cannot write. (params/src/test.rs)
  • Implement the lp_pool contract entrypoints: request_deposit, request_withdraw, process_epoch (epoch finalization), claim, and read-only share_price / pending_* views. Deposits and withdrawals requested during an epoch are only priced at the next finalized epoch. Mint/burn shares through the pinned lp_token (pool is the only minter). Deploy pooled capital as option collateral through options_market's writer flow, accounted against the corresponding vault tag so pool funds are never commingled with individual writers' collateral. Prevent withdrawals from being filled from collateral still locked against a live series. Compute share price at finalization from total assets minus locked collateral over total shares (never from the raw token balance). Emit events for deposit request, withdraw request, epoch finalization, and claim. Handle first epoch with zero shares, epochs where locked collateral exceeds withdrawable assets, claims after several epochs, rounding dust on share burn, and re-requesting in the same epoch. (lp_pool/src/lib.rs)
  • Add unit tests for the lp_pool queue lifecycle and share-price math including rounding, plus an integration test with options_market and vault showing a pool-written series blocking a withdrawal until settlement. Cover first-epoch zero shares, locked collateral exceeding withdrawable assets, multi-epoch claims, dust on share burn, and same-epoch re-requests. (lp_pool/src/test.rs)
  • Update docs/lp_pool.md to document the implemented entrypoints (request_deposit, request_withdraw, process_epoch, claim, share_price, pending_* views), the epoch pricing rule, the share-price formula based on total assets minus locked collateral over total shares, the lp_token mint/burn flow, and how pool capital is deployed through options_market and tagged in vault so it is never commingled with individual writers' collateral. (docs/lp_pool.md)
  • Add a transfer_recipient(stream_id, new_recipient) entrypoint to the streams contract. Require authorization from both the current recipient and the new recipient. Leave the stream's sender, token, amount, and schedule unchanged; a sender cannot unilaterally reassign a recipient. Amounts already withdrawable at transfer time stay claimable by the new recipient (schedule untouched), and the previously withdrawable amount is checkpointed in the event for auditability. Reindex the stream under the new recipient atomically (update any per-recipient list if present). Fail with distinct errors for non-existent streams and completed/canceled streams. Emit an event with stream id, old recipient, new recipient, and withdrawable amount at transfer time. Decide and document behavior for transferring to the current recipient (no-op or explicit rejection). (streams/src/lib.rs)
  • Add the new error variants needed by transfer_recipient: a distinct error for a completed/canceled stream (reuse StreamCanceled if appropriate) and any error for self-transfer if that path is rejected. Keep existing codes stable. (streams/src/error.rs)
  • Add a recipient_transferred event helper carrying stream id, old recipient, new recipient, and the withdrawable amount at transfer time. Keep the existing transferred event for backward compatibility or replace it consistently with the new entrypoint. (streams/src/events.rs)
  • Add unit tests for transfer_recipient: happy path with both auths present, negative tests where only one of the two signatures is provided, where the stream does not exist, and where the stream is complete/canceled. Also test that the old recipient cannot withdraw after the transfer in the same ledger and that the withdrawable amount is checkpointed in the event. (streams/src/test.rs)

Co-authored-by: Simple Agent <agent@simple.dev>
@drips-wave

drips-wave Bot commented Oct 22, 2026

Copy link
Copy Markdown

@chideraisiguzor Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@techisigu
techisigu merged commit 00aa904 into Zenith-options:main Oct 24, 2026
1 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants