From 47f1252ba58a2ff7231f44e7d2d37cfc08796f12 Mon Sep 17 00:00:00 2001 From: adajala Date: Wed, 24 Jun 2026 11:35:37 +0100 Subject: [PATCH] feat: implement MEV protection and commit-reveal for subscription charges --- contracts/subscription/THREAT_MODEL.md | 5 +- contracts/subscription/src/lib.rs | 85 +++++++++++++++++++++++++- contracts/subscription/src/test.rs | 54 ++++++++++++++++ contracts/types/src/lib.rs | 13 ++++ 4 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 contracts/subscription/src/test.rs diff --git a/contracts/subscription/THREAT_MODEL.md b/contracts/subscription/THREAT_MODEL.md index 515829f6..2362a26b 100644 --- a/contracts/subscription/THREAT_MODEL.md +++ b/contracts/subscription/THREAT_MODEL.md @@ -4,8 +4,11 @@ * **Cross-Contract Reentrancy:** A malicious ERC20/Soroban token contract could hijack the `transfer_from` call within `charge()` to recursively call `charge()`, draining the subscriber's balance. * **Read-Only Reentrancy:** An external contract queries `get_subscription()` during a `transfer_from` hook before the state is updated, utilizing stale billing data for parallel logic manipulation. * **Flash Loan Attacks:** Flash loans could be utilized to manipulate liquidity pools if dynamic subscription pricing was based on real-time AMM quotes. +* **Miner Extractable Value (MEV):** Subscription charges broadcasted to the public mempool can be frontrun by bots manipulating oracle prices (sandwiching) or stealing caller rewards by replicating the charge transaction with higher gas. ## 2. Mitigation Strategies Implemented 1. **RAII Reentrancy Guard:** A dedicated `ReentrancyGuard` locks the instance storage upon entering the `charge()` function and explicitly drops it upon exit. 2. **Checks-Effects-Interactions (CEI):** Refactored the `charge` lifecycle to completely update the `next_billing_date` and commit the subscription state *before* interacting with the external Token client. -3. **Revert Propagation:** If an interaction fails or a malicious token attempts to pause the transaction, Soroban's native environment reverts all state, un-doing the effect phase securely. \ No newline at end of file +3. **Revert Propagation:** If an interaction fails or a malicious token attempts to pause the transaction, Soroban's native environment reverts all state, un-doing the effect phase securely. +4. **MEV Commit-Reveal Scheme:** For charges exceeding the `LargeChargeThreshold`, the contract requires a two-step `commit_charge` -> `reveal_charge` execution. The caller commits to a hash of the charge parameters (including intended gas bid and private mempool flag), preventing frontrunners from modifying parameters or reliably sandwiching large payments. +5. **Private Mempool & Gas Monitoring:** Clients can optionally route transactions through private RPC endpoints to hide them from the public mempool. During the reveal phase, expected gas bids are logged on-chain (`mev_monitoring` event), allowing off-chain analysis tools to detect discrepancies between expected and actual network bids (indicating potential extraction). \ No newline at end of file diff --git a/contracts/subscription/src/lib.rs b/contracts/subscription/src/lib.rs index 0b18fa2b..b0d064ae 100644 --- a/contracts/subscription/src/lib.rs +++ b/contracts/subscription/src/lib.rs @@ -4,10 +4,13 @@ mod gas_optimization; mod gas_profiler; mod gas_storage; mod revenue; -use soroban_sdk::{token, Address, Env, IntoVal, String, Symbol, TryFromVal, Val, Vec}; +#[cfg(test)] +mod test; + +use soroban_sdk::{token, Address, BytesN, Env, IntoVal, String, Symbol, TryFromVal, Val, Vec}; use subtrackr_oracle::{OracleError, SubTrackrOracleClient}; use subtrackr_types::{ - Interval, Invoice, Permission, Plan, PriceBounds, StorageKey, Subscription, SubscriptionStatus, + ChargeCommitment, Interval, Invoice, Permission, Plan, PriceBounds, StorageKey, Subscription, SubscriptionStatus, TimeRange, }; @@ -659,6 +662,21 @@ impl SubTrackrSubscription { .unwrap_or(MAX_PLANS_PER_MERCHANT) } + // ── MEV Protection Admin ── + + pub fn set_large_charge_threshold(env: Env, proxy: Address, storage: Address, threshold: i128) { + proxy.require_auth(); + let admin = get_admin(&env, &storage); + admin.require_auth(); + assert!(threshold > 0, "Threshold must be positive"); + storage_instance_set(&env, &storage, StorageKey::LargeChargeThreshold, threshold); + } + + pub fn get_large_charge_threshold(env: Env, proxy: Address, storage: Address) -> i128 { + proxy.require_auth(); + storage_instance_get(&env, &storage, StorageKey::LargeChargeThreshold).unwrap_or(10_000_000_000) // Default 1000 units + } + // ── Plan Management ── pub fn create_plan( @@ -981,7 +999,40 @@ impl SubTrackrSubscription { // ── Payment Processing ── + pub fn commit_charge(env: Env, proxy: Address, storage: Address, subscription_id: u64, hash: BytesN<32>) { + proxy.require_auth(); + let sub: Subscription = + storage_persistent_get(&env, &storage, StorageKey::Subscription(subscription_id)) + .expect("Subscription not found"); + + if sub.subscriber != get_admin(&env, &storage) { + enforce_rate_limit(&env, &storage, &sub.subscriber, "commit_charge"); + } + sub.subscriber.require_auth(); + + let now = env.ledger().timestamp(); + let commitment = ChargeCommitment { + hash, + committed_at: now, + }; + + // Store commitment temporarily for a generous timeframe, e.g., 100 ledgers (~8 mins) + let ttl_ledgers = 100; + storage_temporary_set(&env, &storage, StorageKey::TmpChargeCommitment(subscription_id), commitment, ttl_ledgers); + } + pub fn charge_subscription(env: Env, proxy: Address, storage: Address, subscription_id: u64) { + Self::reveal_charge(env, proxy, storage, subscription_id, 0, false); + } + + pub fn reveal_charge( + env: Env, + proxy: Address, + storage: Address, + subscription_id: u64, + expected_gas_bid: i128, + is_private_mempool: bool, + ) { proxy.require_auth(); let mut sub: Subscription = storage_persistent_get(&env, &storage, StorageKey::Subscription(subscription_id)) @@ -1015,6 +1066,36 @@ impl SubTrackrSubscription { let charge_price = resolve_charge_price(&env, &storage, &plan); + // ── MEV Protection: Commit-Reveal & Gas Analysis ── + let threshold = storage_instance_get(&env, &storage, StorageKey::LargeChargeThreshold) + .unwrap_or(10_000_000_000); + + if charge_price >= threshold { + let commitment_opt: Option = + storage_temporary_get(&env, &storage, StorageKey::TmpChargeCommitment(subscription_id)); + let commitment = commitment_opt.expect("Large charge requires prior commit_charge"); + + let mut payload = soroban_sdk::Bytes::new(&env); + payload.append(&soroban_sdk::Bytes::from_array(&env, &subscription_id.to_be_bytes())); + payload.append(&soroban_sdk::Bytes::from_array(&env, &expected_gas_bid.to_be_bytes())); + let is_priv = if is_private_mempool { 1u8 } else { 0u8 }; + payload.append(&soroban_sdk::Bytes::from_array(&env, &[is_priv])); + + let expected_hash = env.crypto().sha256(&payload); + assert!(commitment.hash == expected_hash.into(), "Commitment hash mismatch"); + + // Prevent commitment reuse + storage_temporary_remove(&env, &storage, StorageKey::TmpChargeCommitment(subscription_id)); + } + + // Gas price analysis for attack detection + if expected_gas_bid > 0 { + env.events().publish( + (String::from_str(&env, "mev_monitoring"), subscription_id), + (expected_gas_bid, is_private_mempool, now), + ); + } + token::Client::new(&env, &plan.token).transfer( &sub.subscriber, &plan.merchant, diff --git a/contracts/subscription/src/test.rs b/contracts/subscription/src/test.rs new file mode 100644 index 00000000..ebe65f12 --- /dev/null +++ b/contracts/subscription/src/test.rs @@ -0,0 +1,54 @@ +#![cfg(test)] + +use super::*; +use soroban_sdk::{testutils::Address as _, Address, Bytes, BytesN, Env}; +use subtrackr_types::{ChargeCommitment, Interval, Plan}; + +#[test] +fn test_mev_commit_reveal_flow() { + let env = Env::default(); + env.mock_all_auths(); + + let proxy = Address::generate(&env); + let storage = Address::generate(&env); + let admin = Address::generate(&env); + let subscriber = Address::generate(&env); + let merchant = Address::generate(&env); + let token = Address::generate(&env); + + let contract_id = env.register_contract(None, SubTrackrSubscription); + let client = SubTrackrSubscriptionClient::new(&env, &contract_id); + + client.initialize(&proxy, &storage, &admin); + + // Set a low threshold to trigger commit-reveal for this test + client.set_large_charge_threshold(&proxy, &storage, &100); + + // Create plan + let plan_id = client.create_plan(&proxy, &storage, &merchant, &String::from_str(&env, "Premium"), &500, &token, &Interval::Monthly); + + // Subscribe + let sub_id = client.subscribe(&proxy, &storage, &subscriber, &plan_id); + + // Setup commit + let expected_gas_bid: i128 = 1000; + let is_private_mempool = true; + + let mut payload = Bytes::new(&env); + payload.append(&Bytes::from_array(&env, &sub_id.to_be_bytes())); + payload.append(&Bytes::from_array(&env, &expected_gas_bid.to_be_bytes())); + let is_priv = if is_private_mempool { 1u8 } else { 0u8 }; + payload.append(&Bytes::from_array(&env, &[is_priv])); + + let hash: BytesN<32> = env.crypto().sha256(&payload).into(); + + client.commit_charge(&proxy, &storage, &sub_id, &hash); + + // Reveal + client.reveal_charge(&proxy, &storage, &sub_id, &expected_gas_bid, &is_private_mempool); + + // Check it succeeded (no panic) + // Next attempt without commit should fail + let res = client.try_reveal_charge(&proxy, &storage, &sub_id, &expected_gas_bid, &is_private_mempool); + assert!(res.is_err()); +} diff --git a/contracts/types/src/lib.rs b/contracts/types/src/lib.rs index a84a9cc8..7cfd98d6 100644 --- a/contracts/types/src/lib.rs +++ b/contracts/types/src/lib.rs @@ -732,6 +732,19 @@ pub enum StorageKey { /// Global maximum number of plans a merchant can create. /// Stored in instance storage; if unset, the implementation default applies. MaxPlansPerMerchant, + + // ── Added in storage version 8 (MEV Protection) ── + /// Tmp charge commitment hash for a pending large charge (subscription_id -> hash) + TmpChargeCommitment(u64), + /// Admin-configurable threshold for when a commit-reveal is required. + LargeChargeThreshold, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct ChargeCommitment { + pub hash: BytesN<32>, + pub committed_at: u64, } /// Slippage protection bounds for oracle-based pricing.