diff --git a/lib/node/mod.rs b/lib/node/mod.rs index f67953a0..baa0c5be 100644 --- a/lib/node/mod.rs +++ b/lib/node/mod.rs @@ -800,6 +800,7 @@ where let mut spent_utxos = HashSet::new(); let mut cumulative_market_states: HashMap> = HashMap::new(); + let mut cumulative_amplify: HashMap = HashMap::new(); for transaction in combined_txs { let txid = transaction.transaction.txid(); @@ -835,6 +836,7 @@ where &rwtxn, &filled_transaction, &mut cumulative_market_states, + &mut cumulative_amplify, ) { Ok(true) => {} Ok(false) => { @@ -894,12 +896,18 @@ where /// For Trade transactions, this checks the cost/proceeds against /// the cumulative market state (accounting for prior txs in this block) and updates /// the cumulative state if the check passes. + /// + /// `cumulative_amplify` tracks the `AmplifyBeta` deposits already selected + /// for this block, per market, so that beta matches connect time. fn check_trade_slippage( &self, rotxn: &sneed::RoTxn, filled_tx: &Authorized, cumulative_states: &mut HashMap>, + cumulative_amplify: &mut HashMap, ) -> Result { + use crate::math::trading::TRADE_MINER_FEE_SATS; + let tx_data = match &filled_tx.transaction.transaction.data { Some(data) => data, None => return Ok(true), // Non-data txs always pass @@ -938,7 +946,18 @@ where } }; - let beta = self.derive_market_beta(&market)?; + // Must mirror `state::block::running_market_state`, which folds + // the `AmplifyBeta` deposits already applied in this block into + // the liquidity base before deriving beta. Pricing against the + // confirmed-only base here would let the miner credit + // TRADE_MINER_FEE_SATS for a tx that connect then skips, and + // self-reject its own block with NotEnoughFees. + let pending_amplify = + cumulative_amplify.get(market_id).copied().unwrap_or(0); + let beta = trading::derive_beta_from_liquidity( + market.liquidity_base_sats.saturating_add(pending_amplify), + market.shares().len(), + ); tracing::debug!( "check_trade_slippage: found market with {} outcomes, beta={}", market.shares().len(), @@ -1038,10 +1057,19 @@ where limit_sats ); - if buy_cost.total_cost_sats > *limit_sats { + // Must mirror the connect-time check in + // `state::block::apply_trade` exactly, otherwise the miner + // credits TRADE_MINER_FEE_SATS for a tx that connect skips + // and self-rejects its own block with NotEnoughFees. + if buy_cost + .total_cost_sats + .saturating_add(TRADE_MINER_FEE_SATS) + > *limit_sats + { tracing::info!( - "Slippage exceeded for buy tx: cost {} sats > max {} sats", + "Slippage exceeded for buy tx: cost {} sats + miner fee {} sats > max {} sats", buy_cost.total_cost_sats, + TRADE_MINER_FEE_SATS, limit_sats ); return Ok(false); @@ -1104,12 +1132,20 @@ where tracing::debug!("check_trade_slippage: Trade passed"); Ok(true) } + TxData::AmplifyBeta { + market_id, amount, .. + } => { + // Track the deposit so later trades on this market in the same + // block are priced at the amplified beta connect will use. + let pending = cumulative_amplify.entry(*market_id).or_insert(0); + *pending = pending.saturating_add(*amount); + Ok(true) + } TxData::ClaimDecision(_) | TxData::CreateMarket { .. } | TxData::SubmitVote { .. } | TxData::SubmitBallot { .. } - | TxData::TransferReputation { .. } - | TxData::AmplifyBeta { .. } => Ok(true), + | TxData::TransferReputation { .. } => Ok(true), } } diff --git a/lib/validation/market.rs b/lib/validation/market.rs index 8a4a780f..b35e0bd2 100644 --- a/lib/validation/market.rs +++ b/lib/validation/market.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use crate::state::Error; use crate::state::decisions::{DecisionId, DecisionType}; @@ -32,6 +32,30 @@ impl MarketValidator { Ok(market_maker_address) } + /// Reject dimension specs that reference the same decision more than + /// once. Mirrors the connect-time check in `generate_state_combos`. + fn validate_no_duplicate_dimensions( + dimension_specs: &[DimensionSpec], + ) -> Result<(), Error> { + let mut seen_decisions = HashSet::new(); + for spec in dimension_specs { + let decision_id = match spec { + DimensionSpec::Single(id) | DimensionSpec::Categorical(id) => { + *id + } + }; + if !seen_decisions.insert(decision_id) { + return Err(Error::InvalidTransaction { + reason: format!( + "Duplicate decision in market dimensions: \ + {decision_id:?}" + ), + }); + } + } + Ok(()) + } + pub fn validate_market_creation( state: &crate::state::State, rotxn: &RoTxn, @@ -108,6 +132,8 @@ impl MarketValidator { }); } + Self::validate_no_duplicate_dimensions(dimension_specs)?; + for spec in dimension_specs { let decision_id = match spec { DimensionSpec::Single(id) | DimensionSpec::Categorical(id) => { @@ -627,6 +653,38 @@ mod tests { assert!(MarketValidator::validate_market_shares(&shares).is_err()); } + #[test] + fn duplicate_dimensions_rejected() { + let a = DecisionId::new(true, 1, 0).unwrap(); + let b = DecisionId::new(true, 1, 1).unwrap(); + + assert!( + MarketValidator::validate_no_duplicate_dimensions(&[ + DimensionSpec::Single(a), + DimensionSpec::Single(b), + ]) + .is_ok() + ); + + assert!( + MarketValidator::validate_no_duplicate_dimensions(&[ + DimensionSpec::Single(a), + DimensionSpec::Single(a), + ]) + .is_err() + ); + + // Same decision referenced through different spec kinds is still a + // duplicate, matching `generate_state_combos`. + assert!( + MarketValidator::validate_no_duplicate_dimensions(&[ + DimensionSpec::Single(a), + DimensionSpec::Categorical(a), + ]) + .is_err() + ); + } + #[test] fn state_transition_valid() { assert!( diff --git a/lib/validation/vote.rs b/lib/validation/vote.rs index 063b99b3..39e31a0e 100644 --- a/lib/validation/vote.rs +++ b/lib/validation/vote.rs @@ -125,6 +125,28 @@ impl VoteValidator { Ok(()) } + /// Reject a transaction whose declared `voting_period` disagrees with the + /// period deterministically derived from the decision id. Mirrors the + /// connect-time check in `apply_submit_vote`. + fn validate_declared_vote_period( + decision_id: crate::state::decisions::DecisionId, + declared_period: u32, + ) -> Result<(), Error> { + let voting_period = decision_id.voting_period(); + if declared_period != voting_period { + return Err(Error::InvalidTransaction { + reason: format!( + "Vote period mismatch: decision {} was claimed in period {} and must be voted on in period {}, but transaction specifies period {}", + decision_id.to_hex(), + decision_id.period_index(), + voting_period, + declared_period + ), + }); + } + Ok(()) + } + fn validate_no_duplicate_vote( state: &crate::state::State, rotxn: &RoTxn, @@ -178,6 +200,13 @@ impl VoteValidator { Self::validate_voter_eligibility(state, rotxn, &voter_address)?; let decision_id = DecisionId::from_bytes(vote_data.decision_id_bytes)?; + + // Voting period is deterministically derived from decision: voting_period = period_index + 1 + Self::validate_declared_vote_period( + decision_id, + vote_data.voting_period, + )?; + let decision = Self::validate_decision_entry(state, rotxn, decision_id)?; @@ -185,7 +214,6 @@ impl VoteValidator { Self::validate_voting_period(state, rotxn, decision_id)?; - // Voting period is deterministically derived from decision: voting_period = period_index + 1 let period_id = VotingPeriodId::new(decision_id.voting_period()); Self::validate_no_duplicate_vote( state, @@ -307,12 +335,39 @@ impl VoteValidator { let mut seen_votes = std::collections::HashSet::<(VotingPeriodId, DecisionId)>::new(); + let mut expected_voting_period: Option = None; for (idx, vote_item) in ballot_data.votes.iter().enumerate() { let decision_id = DecisionId::from_bytes(vote_item.decision_id_bytes)?; // Voting period is deterministically derived from decision: voting_period = period_index + 1 - let period_id = VotingPeriodId::new(decision_id.voting_period()); + let voting_period = decision_id.voting_period(); + + if let Some(expected) = expected_voting_period { + if voting_period != expected { + return Err(Error::InvalidTransaction { + reason: format!( + "Ballot period mismatch: decision {} requires period {} but ballot expects period {}", + decision_id.to_hex(), + voting_period, + expected + ), + }); + } + } else { + expected_voting_period = Some(voting_period); + + if ballot_data.voting_period != voting_period { + return Err(Error::InvalidTransaction { + reason: format!( + "Ballot period mismatch: decisions require period {} but transaction specifies period {}", + voting_period, ballot_data.voting_period + ), + }); + } + } + + let period_id = VotingPeriodId::new(voting_period); if !seen_votes.insert((period_id, decision_id)) { return Err(Error::InvalidTransaction { @@ -542,6 +597,29 @@ mod tests { assert!(VoteValidator::validate_vote_value(&d, 1.5).is_err()); } + #[test] + fn declared_vote_period_matching_derived_is_accepted() { + use crate::state::decisions::DecisionId; + + let decision_id = DecisionId::new(true, 1, 0).unwrap(); + assert_eq!(decision_id.voting_period(), 2); + assert!( + VoteValidator::validate_declared_vote_period(decision_id, 2) + .is_ok() + ); + } + + #[test] + fn declared_vote_period_mismatch_is_rejected() { + use crate::state::decisions::DecisionId; + + let decision_id = DecisionId::new(true, 1, 0).unwrap(); + assert!( + VoteValidator::validate_declared_vote_period(decision_id, 999) + .is_err() + ); + } + #[test] fn transfer_amount_at_precision_is_accepted() { use crate::math::voting::constants::round_reputation;