Feature/active options positions by zahorodnyi · Pull Request #57 · Blockstream/simplicity-dex

zahorodnyi · 2026-02-11T16:57:15Z

Implement a basic feature to display users’ active options positions.

Resolve: #41 (comment)

The feature currently displays the following information about active options:

Table index
User's option tokens amount
User's grantor tokens amount
Contract expiry time
Current BTC price
Contract ID

zahorodnyi · 2026-02-11T17:02:03Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps · 2026-02-11T17:33:49Z

Greptile Summary

This PR implements a new "Active Options Positions" feature that displays users' active options contracts with their token holdings, expiry times, and current BTC price. The implementation adds comprehensive data aggregation logic that queries option/grantor tokens, checks tokens locked in offers, validates contract collateral, and filters for active contracts where other parties hold positions.

Key changes:

Added ContractState struct to aggregate contract state including user holdings and total collateral
Implemented query_contract_tokens_in_offers to track tokens locked as collateral in option-offer contracts
Created multi-step pipeline: parse contracts → aggregate balances → query offers → validate collateral → filter active
Added ActiveOptionsDisplay table with columns for token amounts, expiry, BTC price, and contract ID
Addresses previous issue by mapping both option AND grantor tokens to contracts (positions.rs:345-347)

One potential division-by-zero issue was flagged in is_active() method, but it's mitigated by the is_valid() check that runs first in the filter chain.

Confidence Score: 4/5

This PR is safe to merge with minor risk - the core logic is sound but has one edge case that's currently protected by call-site ordering
The implementation is well-structured with clear separation of concerns and comprehensive state aggregation. The division-by-zero issue in is_active() is currently safe due to filter ordering, but could be more robust with explicit validation. The feature addresses previous review feedback about grantor token mapping. Single logical commit follows best practices.
Pay attention to crates/cli-client/src/cli/positions.rs line 47-50 where division occurs - ensure is_valid() always runs before is_active() in any future refactoring

Important Files Changed

Filename	Overview
crates/cli-client/src/cli/positions.rs	Adds active options positions display with new data structures and aggregation logic. One potential division-by-zero issue flagged but mitigated by call-site filtering.
crates/cli-client/src/cli/tables.rs	Simple addition of table rendering for `ActiveOptionsDisplay` struct. Clean implementation following existing patterns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[run_positions] --> B[Fetch BTC Price]
    A --> C[Get Option Tokens from Wallet]
    A --> D[Get Grantor Tokens from Wallet]
    
    C --> E[build_active_options_displays]
    D --> E
    B --> E
    
    E --> F[fetch_active_contract_states]
    
    F --> G[Parse Option Contracts]
    G --> H[Build token_to_contract Map]
    
    F --> I[Aggregate Token Balances]
    C --> I
    D --> I
    
    F --> J[query_contract_tokens_in_offers]
    H --> J
    J --> K[map_offers_to_contracts]
    
    F --> L[query_collateral_for_candidates]
    
    I --> M[build_and_filter_contract_states]
    K --> M
    L --> M
    
    M --> N{Filter: is_valid && is_active}
    N -->|Yes| O[ContractState]
    N -->|No| P[Discard]
    
    O --> Q[Sort by Expiry]
    Q --> R[Map to ActiveOptionsDisplay]
    R --> S[display_active_options_table]

_{Last reviewed commit: 7a54d11}

greptile-apps

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-11T17:33:52Z

crates/cli-client/src/cli/positions.rs

+/// Query all option tokens locked in option-offer contracts for multiple asset IDs
+async fn query_all_option_tokens_in_offers(
+    wallet: &crate::wallet::Wallet,


Incorrect offer token accounting

query_all_option_tokens_in_offers is described/used as summing “option tokens locked in option-offer contracts”, but option-offer contracts lock collateral/premium (and later settlement), not option tokens. In this function you filter and sum using OptionOfferArguments::get_collateral_asset_id() and UtxoFilter::asset_id(collateral_id) (positions.rs:237-248), so option_tokens_in_offers is actually collateral-in-offers keyed by the collateral asset id. Downstream, build_maker_positions treats that map as “unsold option tokens” (option_tokens_in_offers.get(&option_asset_id) at positions.rs:371-372), so Maker “sold of total” will be wrong (typically under/over-counting depending on assets).

Also appears at positions.rs:213-274 and positions.rs:367-383.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/cli-client/src/cli/positions.rs Line: 213:215 Comment: **Incorrect offer token accounting** `query_all_option_tokens_in_offers` is described/used as summing “option tokens locked in option-offer contracts”, but option-offer contracts lock *collateral/premium* (and later settlement), not option tokens. In this function you filter and sum using `OptionOfferArguments::get_collateral_asset_id()` and `UtxoFilter::asset_id(collateral_id)` (positions.rs:237-248), so `option_tokens_in_offers` is actually *collateral-in-offers* keyed by the collateral asset id. Downstream, `build_maker_positions` treats that map as “unsold option tokens” (`option_tokens_in_offers.get(&option_asset_id)` at positions.rs:371-372), so Maker “sold of total” will be wrong (typically under/over-counting depending on assets). Also appears at positions.rs:213-274 and positions.rs:367-383. How can I resolve this? If you propose a fix, please make it concise.

zahorodnyi · 2026-02-18T17:02:41Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

crates/cli-client/src/cli/positions.rs

zahorodnyi · 2026-02-18T17:15:47Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-18T17:25:09Z

crates/cli-client/src/cli/positions.rs

+    let (option_balances, grantor_balances, tokens_in_offers) = tokio::join!(
+        async { aggregate_token_balances(option_tokens) },
+        async { aggregate_token_balances(grantor_tokens) },
+        query_option_tokens_in_offers(wallet, &option_asset_ids, network)
+    );


Unnecessary async wrapping of sync functions

aggregate_token_balances is a synchronous, CPU-bound function. Wrapping it in async { ... } and passing it to tokio::join! provides no concurrency benefit — the async block completes immediately on first poll. Only query_option_tokens_in_offers is genuinely async here.

Consider either calling the sync functions directly before the async work, or adding a comment explaining the intent:

Suggested change

let (option_balances, grantor_balances, tokens_in_offers) = tokio::join!(

async { aggregate_token_balances(option_tokens) },

async { aggregate_token_balances(grantor_tokens) },

query_option_tokens_in_offers(wallet, &option_asset_ids, network)

);

let option_balances = aggregate_token_balances(option_tokens);

let grantor_balances = aggregate_token_balances(grantor_tokens);

let tokens_in_offers = query_option_tokens_in_offers(wallet, &option_asset_ids, network).await;

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/cli-client/src/cli/positions.rs Line: 453-457 Comment: **Unnecessary async wrapping of sync functions** `aggregate_token_balances` is a synchronous, CPU-bound function. Wrapping it in `async { ... }` and passing it to `tokio::join!` provides no concurrency benefit — the `async` block completes immediately on first poll. Only `query_option_tokens_in_offers` is genuinely async here. Consider either calling the sync functions directly before the async work, or adding a comment explaining the intent: ```suggestion let option_balances = aggregate_token_balances(option_tokens); let grantor_balances = aggregate_token_balances(grantor_tokens); let tokens_in_offers = query_option_tokens_in_offers(wallet, &option_asset_ids, network).await; ``` <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise.

zahorodnyi · 2026-02-18T17:41:31Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-18T17:47:21Z

crates/cli-client/src/cli/positions.rs

+        let contract_id = tpg_str.clone();
+        option_asset_to_contract.insert(args.option_token(), contract_id.clone());


Missing grantor token to contract mapping

Only args.option_token() is inserted into option_asset_to_contract, but grantor tokens are never mapped. This means the option_asset_ids set passed into query_option_tokens_in_offers will only contain option token asset IDs, not grantor token asset IDs. If a grantor token is ever used as collateral in an option-offer, it won't be discovered, and locked_in_offers will undercount.

Additionally, the map_offers_to_contracts function relies on this same map to resolve asset IDs back to contract IDs — any offer collateral that happens to be a grantor token would be silently dropped.

Consider also mapping the grantor token via get_grantor_token_ids() alongside the option token.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/cli-client/src/cli/positions.rs Line: 344:345 Comment: **Missing grantor token to contract mapping** Only `args.option_token()` is inserted into `option_asset_to_contract`, but grantor tokens are never mapped. This means the `option_asset_ids` set passed into `query_option_tokens_in_offers` will only contain option token asset IDs, not grantor token asset IDs. If a grantor token is ever used as collateral in an option-offer, it won't be discovered, and `locked_in_offers` will undercount. Additionally, the `map_offers_to_contracts` function relies on this same map to resolve asset IDs back to contract IDs — any offer collateral that happens to be a grantor token would be silently dropped. Consider also mapping the grantor token via `get_grantor_token_ids()` alongside the option token. How can I resolve this? If you propose a fix, please make it concise.

crates/cli-client/src/cli/positions.rs

zahorodnyi · 2026-02-19T12:18:03Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

crates/cli-client/src/cli/positions.rs

zahorodnyi requested a review from KyrylR as a code owner February 11, 2026 16:57

zahorodnyi self-assigned this Feb 11, 2026

zahorodnyi added the greptile Mark this PR to be reviewed by Greptile label Feb 11, 2026

greptile-apps bot reviewed Feb 11, 2026

View reviewed changes

zahorodnyi marked this pull request as draft February 12, 2026 15:33

zahorodnyi force-pushed the feature/active-options-positions branch 2 times, most recently from e8d39c9 to 3370cb2 Compare February 18, 2026 16:49

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

crates/cli-client/src/cli/positions.rs Show resolved Hide resolved

crates/cli-client/src/cli/positions.rs Show resolved Hide resolved

zahorodnyi force-pushed the feature/active-options-positions branch from 3370cb2 to 9e7fdd6 Compare February 18, 2026 17:14

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

zahorodnyi force-pushed the feature/active-options-positions branch from 9e7fdd6 to 56cdd41 Compare February 18, 2026 17:40

greptile-apps bot reviewed Feb 18, 2026

View reviewed changes

Implement active options positions feature

7a54d11

zahorodnyi force-pushed the feature/active-options-positions branch from 56cdd41 to 7a54d11 Compare February 19, 2026 12:12

greptile-apps bot reviewed Feb 19, 2026

View reviewed changes

crates/cli-client/src/cli/positions.rs Show resolved Hide resolved

zahorodnyi marked this pull request as ready for review February 19, 2026 12:32

		let contract_id = tpg_str.clone();
		option_asset_to_contract.insert(args.option_token(), contract_id.clone());

Conversation

zahorodnyi commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zahorodnyi commented Feb 11, 2026

Uh oh!

greptile-apps bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

zahorodnyi commented Feb 18, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zahorodnyi commented Feb 18, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

zahorodnyi commented Feb 18, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zahorodnyi commented Feb 19, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

zahorodnyi commented Feb 11, 2026 •

edited

Loading

greptile-apps bot commented Feb 11, 2026 •

edited

Loading