Skip to content

[Low] refactor(pool): share invoice-listing lookup CPIs behind a single call #278

Description

@K1NGD4VID

Context

fund_invoice at contracts/pool/src/lib.rs:203-225 makes three separate invoke_contract calls (get_status, get_face_value, get_discount_bps). Every CPI has fixed overhead.

Maintainability — this pattern is duplicated / hard to reason about and cleaning it up makes every future contributor's life easier without changing behaviour.

Where

File: contracts/pool/src/lib.rs
Lines: 203-225

 201|             .unwrap();
 202| 
 203|         let mut args = Vec::new(&env);
 204|         args.push_back(invoice_id.clone().into_val(&env));
 205|         let invoice_status: u32 =
 206|             env.invoke_contract(&invoice_contract, &Symbol::new(&env, "get_status"), args);
 207|         if invoice_status != 1 {
 208|             // 1 = Listed
 209|             panic_with_error!(&env, PoolError::InvoiceNotListed);
 210|         }
 211| 
 212|         let mut args = Vec::new(&env);
 213|         args.push_back(invoice_id.clone().into_val(&env));
 214|         let face_value: u128 = env.invoke_contract(
 215|             &invoice_contract,
 216|             &Symbol::new(&env, "get_face_value"),
 217|             args,
 218|         );
 219|         let mut args = Vec::new(&env);
 220|         args.push_back(invoice_id.clone().into_val(&env));
 221|         let discount_bps: u32 = env.invoke_contract(
 222|             &invoice_contract,
 223|             &Symbol::new(&env, "get_discount_bps"),
 224|             args,
 225|         );
 226| 
 227|         let funded_amount = face_value * (10000 - discount_bps as u128) / 10000;

Proposed Approach

  1. Confirm the duplication / smell is still present at the referenced line(s).
  2. Introduce the helper / abstraction in a private module or private function — do not export new API surface as part of a refactor.
  3. Migrate every call site in one PR — leaving half-migrated code causes drift.
  4. Confirm behavioural equivalence with cargo test --workspace; the diff should not touch any test file except cosmetically.

Acceptance Criteria

  • Add get_funding_terms(invoice_id) -> (status, face_value, discount_bps) on the invoice contract
  • Pool calls it once instead of three times
  • Update tests
  • No behaviour regression in existing tests (cargo test --workspace passes).
  • No new clippy warnings introduced by this change.

Definition of Done

  • PR links back to this issue with Closes #<this-issue-number> in the description.
  • CI (build-and-test) is green on the PR.
  • For contract changes: cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, and cargo test --workspace all pass locally.
  • New behaviour is covered by at least one test (positive path) and one negative test where applicable.
  • Any new public function has rustdoc documenting # Arguments, # Auth, # Panics, and # Returns.

Contributor Tips

  • Contracts live under contracts/{registry,invoice,escrow,pool}/src/. Each is a workspace member of the root Cargo.toml.
  • Test locally with cargo test -p trusttrove-<contract> for fast iteration, or cargo test --workspace before pushing.
  • Follow the existing pattern for typed errors: define the variant in errors.rs, panic with panic_with_error!(&env, MyContractError::Variant).
  • After any persistent().set(&key, &value) remember to .extend_ttl(&key, THRESHOLD, EXTEND_TO) — see the TTL constants proposal in a companion issue.

Related Issues

Other tickets in the same scope (pool): #253, #254, #255, #256

If you spot overlap while working, drop a comment here and I'll re-scope or link them.

Tech Stack

contracts (Rust / Soroban)

How to Claim

Comment .take (or just say you're working on it) and open a PR that includes Closes #278 in the description.

Difficulty: Low (complexity:low) — used for Drips Wave point allocation.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions