Skip to content

Conversation

@KyrylR
Copy link
Collaborator

@KyrylR KyrylR commented Jan 12, 2026

No description provided.

@KyrylR KyrylR added the greptile Mark this PR to be reviewed by Greptile label Jan 12, 2026
@KyrylR
Copy link
Collaborator Author

KyrylR commented Jan 12, 2026

@greptileai

@greptile-apps
Copy link

greptile-apps bot commented Jan 12, 2026

Greptile Overview

Greptile Summary

Overview

This PR migrates the simplicity-dex CLI client to use a fixed option contract by updating the simplicity-contracts dependency from revision 94993a0 to 1a692a6. The changes adapt the codebase to API modifications in the underlying contracts library while also introducing improvements to fee handling and transaction construction.

Key Changes

Dependency Update (Cargo.toml)

  • Updates contracts and cli-helper packages to revision 1a692a6
  • This update brings the "fixed option contract" implementation

API Compatibility Updates (option.rs)

The PR adapts to breaking API changes in the contracts library:

  • build_option_funding: Now accepts &blinding_keypair (full keypair) instead of &blinding_keypair.public_key()
  • finalize_options_transaction: All branch parameters changed from owned values to references (option_branch&option_branch, branch&branch)

Fee Handling Improvements (option.rs)

  • Separated fee calculations: Creation transaction fee (creation_fee) is now calculated independently from funding transaction fee (funding_fee), providing more accurate fee estimation for each transaction
  • Added funding fee estimation: New code block (lines 196-239) estimates the funding transaction fee by building, finalizing, and signing a test transaction, then calculating the actual fee based on the signed weight
  • User-friendly output now shows both "Creation fee: X sats" and "Funding fee: Y sats" separately

LBTC Settlement Optimization (option.rs, Exercise command)

When the settlement asset is LBTC (Liquid Bitcoin), the code now optimizes UTXO selection:

  • Before: Always queried for separate fee and settlement UTXOs
  • After: Uses a single combined UTXO when both are LBTC, reducing transaction inputs and complexity
  • Properly handles the optional fee_input throughout fee estimation and transaction construction

Code Cleanup

  • Removed debug statement dbg!("here"); from the Expire command (line 702)
  • Improved NOSTR event publication message clarity

Commit Structure Concern

⚠️ Policy Violation: The PR contains 2 commits that update Cargo.toml incrementally:

  1. First commit: Updates to revision 22bddb0
  2. Second commit: Updates to revision 1a692a6

This violates the repository's custom instruction requiring "logical, independent commits that each build and pass unit tests". The first commit likely references an intermediate/incomplete version of the contracts library, and may not build or function correctly on its own. The commits should be squashed or restructured so each commit is independently buildable.

Testing Recommendations

  • Verify that option creation, funding, exercise, expiry, settlement, and cancellation flows all work correctly with the new contract version
  • Test the LBTC settlement optimization path specifically
  • Confirm fee estimates are accurate for both creation and funding transactions
  • Ensure the separate fee handling doesn't cause issues when explicit fees are provided

Confidence Score: 4/5

  • This PR is generally safe to merge with one structural concern about commit history
  • The code changes are functionally correct and improve the option contract handling by properly separating creation and funding fees, optimizing LBTC settlement, and updating to the fixed contract API. However, the commit structure violates the repository's policy requiring independent, buildable commits - commit 1 updates to an intermediate dependency version (22bddb0) that commit 2 then updates again (1a692a6), suggesting commit 1 may not build independently.
  • Cargo.toml needs attention due to incremental dependency updates across commits

Important Files Changed

File Analysis

Filename Score Overview
Cargo.toml 4/5 Updates simplicity-contracts dependency from rev 94993a0 to 1a692a6, incrementally updated across commits
crates/cli-client/src/cli/option.rs 4/5 Migrates to fixed option contract with API changes, adds separate fee estimation for creation/funding, optimizes LBTC handling, removes debug statement

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as CLI Client
    participant Wallet
    participant Contracts as Contracts SDK
    participant Blockchain

    Note over User,Blockchain: Option Creation Flow (Updated)
    
    User->>CLI: option create --collateral-asset X --total-collateral Y
    CLI->>Wallet: Query 3 LBTC UTXOs for fees
    Wallet-->>CLI: LBTC UTXOs (first_fee, second_fee, funding_fee)
    
    alt Collateral is not LBTC
        CLI->>Wallet: Query collateral asset UTXO
        Wallet-->>CLI: Collateral UTXO
    end
    
    Note over CLI,Contracts: Creation Transaction (with separate fee)
    CLI->>Contracts: build_option_creation(first_fee, second_fee)
    CLI->>CLI: Estimate creation_fee from signed weight
    CLI->>Contracts: build_option_creation(with creation_fee)
    Contracts-->>CLI: Creation PST + TaprootPubkeyGen
    CLI->>CLI: Extract, unblind, sign transaction
    CLI->>CLI: Create option_token_utxo and grantor_token_utxo
    
    Note over CLI,Contracts: Funding Transaction (NEW: separate fee estimation)
    CLI->>Contracts: build_option_funding(&blinding_keypair, tokens, collateral)
    CLI->>CLI: Estimate funding_fee from signed weight
    CLI->>Contracts: build_option_funding(with funding_fee)
    Contracts-->>CLI: Funding PST + option_branch
    CLI->>CLI: Finalize with &option_branch (reference)
    CLI->>CLI: Sign transaction
    
    CLI->>Blockchain: Broadcast creation_tx
    CLI->>Blockchain: Broadcast funding_tx
    
    Note over User,Blockchain: Option Exercise Flow (Updated)
    
    User->>CLI: option exercise
    CLI->>Wallet: Query option tokens
    Wallet-->>CLI: Option token entries
    
    alt Settlement asset is LBTC (NEW optimization)
        CLI->>Wallet: Query single UTXO for (settlement + fee)
        Wallet-->>CLI: Combined LBTC UTXO
        Note over CLI: fee_input = None
    else Settlement asset is different
        CLI->>Wallet: Query settlement UTXO
        CLI->>Wallet: Query fee UTXO separately
        Wallet-->>CLI: Settlement and Fee UTXOs
        Note over CLI: fee_input = Some(fee_utxo)
    end
    
    CLI->>Wallet: Query collateral at contract address
    Wallet-->>CLI: Collateral UTXO
    
    CLI->>Contracts: build_option_exercise(collateral, option, settlement, fee_input)
    CLI->>CLI: Estimate actual_fee from signed weight
    CLI->>Contracts: build_option_exercise(with actual_fee)
    Contracts-->>CLI: Exercise PST + option_branch
    CLI->>CLI: Finalize with &option_branch (reference)
    CLI->>CLI: Sign with proper input offset
    CLI->>Blockchain: Broadcast exercise_tx
Loading

@KyrylR KyrylR force-pushed the fix/creation-and-fund branch from ac13f19 to 70a2480 Compare January 12, 2026 15:07
@KyrylR KyrylR force-pushed the fix/creation-and-fund branch from 70a2480 to 9c9bf69 Compare January 12, 2026 15:07
@KyrylR KyrylR merged commit fb51967 into main Jan 12, 2026
1 check passed
@KyrylR KyrylR deleted the fix/creation-and-fund branch January 12, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

greptile Mark this PR to be reviewed by Greptile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants