From 343370758a705e0cb146f306950d6592250b1f1e Mon Sep 17 00:00:00 2001 From: drcpu Date: Thu, 23 Jan 2025 19:18:09 +0000 Subject: [PATCH 01/13] chore(config): set protocol version details for rc.10 --- config/src/defaults.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/config/src/defaults.rs b/config/src/defaults.rs index 2d80d125b..154bfaa02 100644 --- a/config/src/defaults.rs +++ b/config/src/defaults.rs @@ -563,4 +563,14 @@ impl Defaults for Testnet { fn consensus_constants_minimum_difficulty(&self) -> u32 { 0 } + + fn protocol_versions(&self) -> HashMap { + [ + (ProtocolVersion::V1_7, (0, 45)), + (ProtocolVersion::V1_8, (101, 45)), + (ProtocolVersion::V2_0, (281, 20)), + ] + .into_iter() + .collect() + } } From 40f2c721e54aa68c056632fadf615d3e44e15aa6 Mon Sep 17 00:00:00 2001 From: drcpu Date: Thu, 23 Jan 2025 19:18:48 +0000 Subject: [PATCH 02/13] fix(data_structures): add to total_ut_weight instead of total_st_weight --- data_structures/src/chain/mod.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/data_structures/src/chain/mod.rs b/data_structures/src/chain/mod.rs index d73f6b65c..6fcab4f87 100644 --- a/data_structures/src/chain/mod.rs +++ b/data_structures/src/chain/mod.rs @@ -3450,15 +3450,7 @@ impl TransactionsPool { if fee < self.minimum_ut_fee { return vec![Transaction::Unstake(ut_tx)]; } else { - self.total_st_weight += u64::from(ut_tx.weight()); - - // TODO - // for input in &ut_tx.body.inputs { - // self.output_pointer_map - // .entry(input.output_pointer) - // .or_insert_with(Vec::new) - // .push(ut_tx.hash()); - // } + self.total_ut_weight += u64::from(ut_tx.weight()); self.ut_transactions.insert(key, (priority, ut_tx)); self.sorted_ut_index.insert((priority, key)); From e454bcecb07a8a4a6d6e8f65368583fcd77b6545 Mon Sep 17 00:00:00 2001 From: drcpu Date: Thu, 23 Jan 2025 19:19:26 +0000 Subject: [PATCH 03/13] chore(node): cargo fmt --- node/src/actors/chain_manager/handlers.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/node/src/actors/chain_manager/handlers.rs b/node/src/actors/chain_manager/handlers.rs index 46e89352f..6356f54b1 100644 --- a/node/src/actors/chain_manager/handlers.rs +++ b/node/src/actors/chain_manager/handlers.rs @@ -1474,11 +1474,7 @@ impl Handler for ChainManager { let mut powers = HashMap::new(); let current_epoch = self.current_epoch.unwrap(); for capability in msg.capabilities { - for (key, power) in self - .chain_state - .stakes - .by_rank(capability, current_epoch) - { + for (key, power) in self.chain_state.stakes.by_rank(capability, current_epoch) { powers.entry(key).or_insert(vec![]).push(power); } } From 4aa3483b4b105dabaa853776de579fc33e0ffc69 Mon Sep 17 00:00:00 2001 From: drcpu Date: Thu, 23 Jan 2025 19:21:16 +0000 Subject: [PATCH 04/13] chore(node): make future SyncTarget assertion more verbose --- node/src/actors/chain_manager/handlers.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/node/src/actors/chain_manager/handlers.rs b/node/src/actors/chain_manager/handlers.rs index 6356f54b1..7bf9b1383 100644 --- a/node/src/actors/chain_manager/handlers.rs +++ b/node/src/actors/chain_manager/handlers.rs @@ -2219,7 +2219,11 @@ where let current_superblock_index = current_epoch / superblock_period; assert!( current_superblock_index >= sync_target.superblock.checkpoint, - "Provided a sync target that is in the future" + "Provided a sync target (epoch: {}, superblock index: {}) that is in the future (epoch: {}, superblock index: {})", + current_epoch, + current_superblock_index, + sync_target.block.checkpoint, + sync_target.superblock.checkpoint, ); // If the chain reverted, this function cannot receive blocks from between the reverted epochs From 98ab9ad92bc32f2e6d431f0e0e5cb20536086c22 Mon Sep 17 00:00:00 2001 From: drcpu Date: Thu, 23 Jan 2025 19:49:33 +0000 Subject: [PATCH 05/13] feat(node): add some sanitity checks to the authorization string of a stake transaction --- node/src/actors/json_rpc/api.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/node/src/actors/json_rpc/api.rs b/node/src/actors/json_rpc/api.rs index 8370282a4..5b69e05ad 100644 --- a/node/src/actors/json_rpc/api.rs +++ b/node/src/actors/json_rpc/api.rs @@ -48,8 +48,8 @@ use crate::{ GetHighestCheckpointBeacon, GetItemBlock, GetItemSuperblock, GetItemTransaction, GetKnownPeers, GetMemoryTransaction, GetMempool, GetNodeStats, GetProtocolInfo, GetReputation, GetSignalingInfo, GetState, GetSupplyInfo, GetSupplyInfo2, GetUtxoInfo, - InitializePeers, IsConfirmedBlock, QueryStakePowers, QueryStakes, QueryStakesFilter, - Rewind, SnapshotExport, SnapshotImport, StakeAuthorization, + InitializePeers, IsConfirmedBlock, MagicEither, QueryStakePowers, QueryStakes, + QueryStakesFilter, Rewind, SnapshotExport, SnapshotImport, StakeAuthorization, }, peers_manager::PeersManager, sessions_manager::SessionsManager, @@ -2069,6 +2069,28 @@ pub async fn stake(params: Result) -> JsonRpcResult { // This is the actual message that gets signed as part of the authorization let msg = withdrawer.as_secp256k1_msg(); + // Perform some sanity checks on the authorization string + match params.authorization { + MagicEither::Left(ref hex_str) => { + // Authorization string is not a hexadecimal string + if !hex_str.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(Error::invalid_params( + "The authorization string is not a hexadecimal string", + )); + } + + // Invalid authorization length + if hex_str.len() != 170 { + return Err(Error::invalid_params(format!( + "Authorization string has an unexpected length: {} != {}", + hex_str.len(), + 170 + ))); + } + } + MagicEither::Right(_) => (), + }; + let authorization = params .authorization .try_do_magic(|hex_str| { From cd5e91d1319f8977c31aa889bb57ef889d601489 Mon Sep 17 00:00:00 2001 From: drcpu Date: Thu, 23 Jan 2025 22:00:18 +0100 Subject: [PATCH 06/13] fix(validations): use the current timestamp to create UTXO's with a relative timestamp --- node/src/actors/chain_manager/mining.rs | 4 ++++ validations/src/tests/mod.rs | 10 +++++++++- validations/src/validations.rs | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/node/src/actors/chain_manager/mining.rs b/node/src/actors/chain_manager/mining.rs index 18c42ea13..286e7c973 100644 --- a/node/src/actors/chain_manager/mining.rs +++ b/node/src/actors/chain_manager/mining.rs @@ -1002,6 +1002,7 @@ pub fn build_block( vt_tx.body.outputs.iter(), vt_tx.hash(), epoch, + epoch_constants, checkpoint_zero_timestamp, ); value_transfer_txns.push(vt_tx.clone()); @@ -1109,6 +1110,7 @@ pub fn build_block( dr_tx.body.outputs.iter(), dr_tx.hash(), epoch, + epoch_constants, checkpoint_zero_timestamp, ); @@ -1243,6 +1245,7 @@ pub fn build_block( st_tx.body.change.iter(), st_tx.hash(), epoch, + epoch_constants, checkpoint_zero_timestamp, ); stake_txns.push(st_tx.clone()); @@ -1286,6 +1289,7 @@ pub fn build_block( vec![&ut_tx.body.withdrawal], ut_tx.hash(), epoch, + epoch_constants, checkpoint_zero_timestamp, ); unstake_txns.push(ut_tx.clone()); diff --git a/validations/src/tests/mod.rs b/validations/src/tests/mod.rs index ed3e2ff50..930099ad8 100644 --- a/validations/src/tests/mod.rs +++ b/validations/src/tests/mod.rs @@ -12866,7 +12866,15 @@ fn validate_update_utxo_diff() { |inputs: &[Input], outputs: &[ValueTransferOutput], block_numbers: &[u32]| { let mut utxo_diff = UtxoDiff::new(&utxo_set, block_number); - update_utxo_diff(&mut utxo_diff, inputs, outputs, tx_hash, 0, 0); + update_utxo_diff( + &mut utxo_diff, + inputs, + outputs, + tx_hash, + 0, + EpochConstants::default(), + 0, + ); let diff = utxo_diff.take_diff(); diff --git a/validations/src/validations.rs b/validations/src/validations.rs index 55e721343..c088de54e 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -1834,6 +1834,7 @@ pub fn update_utxo_diff<'a, IterInputs, IterOutputs>( outputs: IterOutputs, tx_hash: Hash, epoch: Epoch, + epoch_constants: EpochConstants, checkpoint_zero_timestamp: i64, ) where IterInputs: IntoIterator, @@ -1885,10 +1886,14 @@ pub fn update_utxo_diff<'a, IterInputs, IterOutputs>( #[allow(clippy::cast_sign_loss)] let output_to_insert = if get_protocol_version(Some(epoch)) >= ProtocolVersion::V2_0 { if output.time_lock < checkpoint_zero_timestamp.try_into().unwrap() { + let epoch_timestamp = match epoch_constants.epoch_timestamp(epoch) { + Ok((timestamp, _)) => timestamp, + Err(e) => panic!("Failed to get timestamp for epoch {}: {}", epoch, e), + }; ValueTransferOutput { pkh: output.pkh, value: output.value, - time_lock: output.time_lock + checkpoint_zero_timestamp as u64, + time_lock: output.time_lock + epoch_timestamp as u64, } } else { output.clone() @@ -1897,6 +1902,8 @@ pub fn update_utxo_diff<'a, IterInputs, IterOutputs>( output.clone() }; + log::info!("Inserting output {:?}", output_to_insert); + utxo_diff.insert_utxo(output_pointer, output_to_insert, block_number); } } @@ -1994,6 +2001,7 @@ pub fn validate_block_transactions( outputs, transaction.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); @@ -2066,6 +2074,7 @@ pub fn validate_block_transactions( outputs, transaction.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); @@ -2160,6 +2169,7 @@ pub fn validate_block_transactions( outputs, transaction.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); @@ -2287,6 +2297,7 @@ pub fn validate_block_transactions( outputs, transaction.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); @@ -2364,6 +2375,7 @@ pub fn validate_block_transactions( outputs, transaction.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); @@ -2415,6 +2427,7 @@ pub fn validate_block_transactions( outputs, transaction.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); @@ -2445,6 +2458,7 @@ pub fn validate_block_transactions( block.txns.mint.outputs.iter(), block.txns.mint.versioned_hash(block_protocol_version), epoch, + epoch_constants, consensus_constants.checkpoint_zero_timestamp, ); } From f665a2b547cb175f21193ad4042db8376cc30d0b Mon Sep 17 00:00:00 2001 From: drcpu Date: Tue, 28 Jan 2025 08:27:53 +0000 Subject: [PATCH 07/13] feat(stakes): derive nonce from block epoch to guarantee uniqueness --- data_structures/src/staking/helpers.rs | 2 + data_structures/src/staking/stake.rs | 7 +- data_structures/src/staking/stakes.rs | 46 ++++++- node/src/actors/chain_manager/mod.rs | 171 ++++++++++++++++++++++++- validations/src/eligibility/current.rs | 1 + 5 files changed, 221 insertions(+), 6 deletions(-) diff --git a/data_structures/src/staking/helpers.rs b/data_structures/src/staking/helpers.rs index b9426df03..8ffb5e924 100644 --- a/data_structures/src/staking/helpers.rs +++ b/data_structures/src/staking/helpers.rs @@ -373,6 +373,7 @@ where + Default + DeserializeOwned + Display + + From + From + Saturating + Send @@ -457,6 +458,7 @@ where + Default + DeserializeOwned + Display + + From + From + Saturating + Send diff --git a/data_structures/src/staking/stake.rs b/data_structures/src/staking/stake.rs index 351f88b47..dbea9c93d 100644 --- a/data_structures/src/staking/stake.rs +++ b/data_structures/src/staking/stake.rs @@ -24,11 +24,12 @@ where pub epochs: CapabilityMap, /// The amount of stake and unstake actions. pub nonce: Nonce, - // These two phantom fields are here just for the sake of specifying generics. + /// This phantom field is here just for the sake of specifying generics. #[serde(skip)] - phantom_address: PhantomData
, + pub phantom_address: PhantomData
, + /// This phantom field is here just for the sake of specifying generics. #[serde(skip)] - phantom_power: PhantomData, + pub phantom_power: PhantomData, } impl diff --git a/data_structures/src/staking/stakes.rs b/data_structures/src/staking/stakes.rs index a638a6ec5..befdf8932 100644 --- a/data_structures/src/staking/stakes.rs +++ b/data_structures/src/staking/stakes.rs @@ -128,6 +128,7 @@ where + Debug + Default + Display + + From + From + Saturating + Send @@ -200,6 +201,7 @@ where + Debug + Default + Display + + From + From + Saturating + Send @@ -229,7 +231,10 @@ where .entry(key.clone()) .or_insert(SyncStakeEntry::from(StakeEntry { key: key.clone(), - ..Default::default() + value: Stake { + nonce: Nonce::from(epoch), + ..Default::default() + }, })); if !stake_found { @@ -782,6 +787,7 @@ where + Debug + Default + Display + + From + From + Saturating + Send @@ -844,6 +850,7 @@ where + Debug + Default + Display + + From + From + Saturating + Send @@ -904,6 +911,7 @@ where + Debug + Default + Display + + From + From + Saturating + Send @@ -948,6 +956,7 @@ where + Debug + Default + Display + + From + From + Saturating + Send @@ -1674,4 +1683,39 @@ mod tests { ] ); } + + #[test] + fn test_nonce_uniqueness() { + // First, lets create a setup with a few stakers + let mut stakes = StakesTester::default(); + let alice = "Alice"; + let bob = "Bob"; + + let alice_alice = (alice, alice); + let bob_alice = (bob, alice); + + // Add some stake and verify the nonce is ever increasing and unique + stakes + .add_stake(alice_alice, 10, 0, true, MIN_STAKE_NANOWITS) + .unwrap(); + assert_eq!(stakes.query_nonce(alice_alice), Ok(1)); + stakes + .add_stake(bob_alice, 20, 0, true, MIN_STAKE_NANOWITS) + .unwrap(); + assert_eq!(stakes.query_nonce(bob_alice), Ok(1)); + stakes + .remove_stake(bob_alice, 20, true, MIN_STAKE_NANOWITS) + .unwrap(); + stakes + .add_stake(bob_alice, 20, 1, true, MIN_STAKE_NANOWITS) + .unwrap(); + assert_eq!(stakes.query_nonce(bob_alice), Ok(2)); + stakes + .remove_stake(bob_alice, 20, true, MIN_STAKE_NANOWITS) + .unwrap(); + stakes + .add_stake(bob_alice, 20, 2, true, MIN_STAKE_NANOWITS) + .unwrap(); + assert_eq!(stakes.query_nonce(bob_alice), Ok(3)); + } } diff --git a/node/src/actors/chain_manager/mod.rs b/node/src/actors/chain_manager/mod.rs index 993a53457..85c098e4d 100644 --- a/node/src/actors/chain_manager/mod.rs +++ b/node/src/actors/chain_manager/mod.rs @@ -3863,11 +3863,12 @@ mod tests { chain::{ BlockMerkleRoots, BlockTransactions, ChainInfo, DataRequestState, Environment, Input, KeyedSignature, OutputPointer, PartialConsensusConstants, PublicKey, SecretKey, - Signature, ValueTransferOutput, + Signature, StakeOutput, ValueTransferOutput, }, proto::versioning::{ProtocolInfo, VersionedHashable}, transaction::{ - CommitTransaction, DRTransaction, MintTransaction, RevealTransaction, VTTransaction, + CommitTransaction, DRTransaction, MintTransaction, RevealTransaction, StakeTransaction, + StakeTransactionBody, UnstakeTransaction, UnstakeTransactionBody, VTTransaction, VTTransactionBody, }, vrf::BlockEligibilityClaim, @@ -5139,4 +5140,170 @@ mod tests { .coins; assert_eq!(new_coins, 104_005_000_000_000.into()); } + + #[test] + fn test_unstake_unique_nonce() { + register_protocol_version(ProtocolVersion::V2_0, 100, 15); + + // Create inputs + let vti_1 = Input::new( + "1111111111111111111111111111111111111111111111111111111111111111:0" + .parse() + .unwrap(), + ); + let vti_2 = Input::new( + "2222222222222222222222222222222222222222222222222222222222222222:0" + .parse() + .unwrap(), + ); + + // Start epoch + let mut block_epoch = 101; + + // Create addresses and validators + let pkh_1 = pkh(&PRIV_KEY_1); + let pkh_2 = pkh(&PRIV_KEY_2); + + let validator_1 = StakeKey { + validator: pkh_1, + withdrawer: pkh_1, + }; + let validator_2 = StakeKey { + validator: pkh_2, + withdrawer: pkh_2, + }; + + // Create stakes tracker + let mut stakes = StakesTracker::default(); + + let stake_txns_1 = vec![ + StakeTransaction::new( + StakeTransactionBody::new( + vec![vti_1], + StakeOutput { + value: 1_000_000_000_000_000, + key: validator_1.clone(), + ..Default::default() + }, + None, + ), + vec![], + ), + StakeTransaction::new( + StakeTransactionBody::new( + vec![vti_2], + StakeOutput { + value: 2_000_000_000_000_000, + key: validator_2.clone(), + ..Default::default() + }, + None, + ), + vec![], + ), + ]; + + // Process all stake transactions + process_stake_transactions( + &mut stakes, + stake_txns_1.iter(), + block_epoch, + 10_000_000_000_000, + ) + .unwrap(); + + let unstake_txn_1 = UnstakeTransaction::new( + UnstakeTransactionBody::new( + pkh_2, + ValueTransferOutput { + pkh: pkh_2, + value: 2_000_000_000_000_000, + time_lock: 1000, + }, + 0, + stakes.query_nonce(validator_2.clone()).unwrap(), + ), + KeyedSignature::default(), + ); + + // Check unstake nonce + assert_eq!(stakes.query_nonce(validator_2.clone()), Ok(102)); + + // Advance time + block_epoch += 1; + + // Unstake all for validator 2 + process_unstake_transactions( + &mut stakes, + vec![unstake_txn_1.clone()].iter(), + 10_000_000_000_000, + ) + .unwrap(); + + // Check validator 2 is removed from stakes + assert_eq!( + stakes.query_stakes(QueryStakesKey::Key(validator_2.clone())), + Err(StakesError::EntryNotFound { + key: validator_2.clone() + }) + ); + + // Advance time + block_epoch += 1; + + // Create new stake transaction for the same validator 2 and process it + let stake_txns_2 = vec![StakeTransaction::new( + StakeTransactionBody::new( + vec![vti_1], + StakeOutput { + value: 3_000_000_000_000_000, + key: validator_2.clone(), + ..Default::default() + }, + None, + ), + vec![], + )]; + + process_stake_transactions( + &mut stakes, + stake_txns_2.iter(), + block_epoch, + 10_000_000_000_000, + ) + .unwrap(); + + // Advance time + block_epoch += 1; + + let unstake_txn_2 = UnstakeTransaction::new( + UnstakeTransactionBody::new( + pkh_2, + ValueTransferOutput { + pkh: pkh_2, + value: 3_000_000_000_000_000, + time_lock: 1000, + }, + 0, + stakes.query_nonce(validator_2.clone()).unwrap(), + ), + KeyedSignature::default(), + ); + + // Check unstake nonce + assert_eq!(stakes.query_nonce(validator_2.clone()), Ok(104)); + + // Check hashes are unique due to unique nonces + assert_ne!(unstake_txn_1.hash(), unstake_txn_2.hash()); + + // Unstake all again for validator 2 + process_unstake_transactions(&mut stakes, vec![unstake_txn_2].iter(), 10_000_000_000_000) + .unwrap(); + + // Check validator 2 is removed from stakes again + assert_eq!( + stakes.query_stakes(QueryStakesKey::Key(validator_2.clone())), + Err(StakesError::EntryNotFound { key: validator_2 }) + ); + } } diff --git a/validations/src/eligibility/current.rs b/validations/src/eligibility/current.rs index 22c8b3247..e8a067360 100644 --- a/validations/src/eligibility/current.rs +++ b/validations/src/eligibility/current.rs @@ -146,6 +146,7 @@ where + Display + num_traits::Saturating + AddAssign + + From + From + Sync + Send From 93b5d170cae7533cc35b46287f4038533eaf4590 Mon Sep 17 00:00:00 2001 From: drcpu Date: Tue, 28 Jan 2025 08:33:15 +0000 Subject: [PATCH 08/13] chore(validations): temporarily disable nonce checking for testnet under epoch 20_000 --- validations/src/validations.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/validations/src/validations.rs b/validations/src/validations.rs index c088de54e..2ce10a4b7 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -17,9 +17,9 @@ use witnet_data_structures::{ chain::{ tapi::ActiveWips, Block, BlockMerkleRoots, CheckpointBeacon, CheckpointVRF, ConsensusConstants, ConsensusConstantsWit2, DataRequestOutput, DataRequestStage, - DataRequestState, Epoch, EpochConstants, Hash, Hashable, Input, KeyedSignature, - OutputPointer, PublicKeyHash, RADRequest, RADTally, RADType, Reputation, ReputationEngine, - SignaturesToVerify, StakeOutput, ValueTransferOutput, + DataRequestState, Environment, Epoch, EpochConstants, Hash, Hashable, Input, + KeyedSignature, OutputPointer, PublicKeyHash, RADRequest, RADTally, RADType, Reputation, + ReputationEngine, SignaturesToVerify, StakeOutput, ValueTransferOutput, }, data_request::{ calculate_reward_collateral_ratio, calculate_tally_change, calculate_witness_reward, @@ -27,7 +27,7 @@ use witnet_data_structures::{ data_request_has_too_many_witnesses, DataRequestPool, }, error::{BlockError, DataRequestError, TransactionError}, - get_protocol_version, get_protocol_version_activation_epoch, + get_environment, get_protocol_version, get_protocol_version_activation_epoch, proto::versioning::{ProtocolVersion, VersionedHashable}, radon_report::{RadonReport, ReportContext}, staking::prelude::{Power, QueryStakesKey, StakeKey, StakesTracker}, @@ -1497,13 +1497,19 @@ pub fn validate_unstake_transaction<'a>( } // TODO: modify this to enable delegated staking with multiple withdrawer addresses on a single validator - let nonce = stake_entry.first().map(|stake| stake.value.nonce).unwrap(); - if ut_tx.body.nonce != nonce { - return Err(TransactionError::UnstakeInvalidNonce { - used: ut_tx.body.nonce, - current: nonce, + // TODO: remove this check for mainnet release + if (get_environment() == Environment::Development + || get_environment() == Environment::Testnet) + && epoch > 20_000 + { + let nonce = stake_entry.first().map(|stake| stake.value.nonce).unwrap(); + if ut_tx.body.nonce != nonce { + return Err(TransactionError::UnstakeInvalidNonce { + used: ut_tx.body.nonce, + current: nonce, + } + .into()); } - .into()); } staked_amount From 7df7423974f6ebc88f8035c6a2118e2c450fec84 Mon Sep 17 00:00:00 2001 From: drcpu Date: Tue, 28 Jan 2025 12:54:40 +0000 Subject: [PATCH 09/13] feat(mining): add extra stake balance check to unstake transactions before including them in a block --- data_structures/src/chain/mod.rs | 13 ++++++++ node/src/actors/chain_manager/mining.rs | 44 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/data_structures/src/chain/mod.rs b/data_structures/src/chain/mod.rs index 6fcab4f87..19172a819 100644 --- a/data_structures/src/chain/mod.rs +++ b/data_structures/src/chain/mod.rs @@ -3202,6 +3202,19 @@ impl TransactionsPool { } } + /// Remove unstake transactions that would result in understaking on a validator + pub fn remove_understake_transactions(&mut self, transactions: Vec) { + for ut_tx_hash in transactions.iter() { + if let Some(ut_tx) = self + .ut_transactions + .get(ut_tx_hash) + .map(|(_, ut)| ut.clone()) + { + self.ut_remove(&ut_tx); + } + } + } + /// Remove an unstake transaction from the pool. /// /// This should be used to remove transactions that got included in a consolidated block. diff --git a/node/src/actors/chain_manager/mining.rs b/node/src/actors/chain_manager/mining.rs index 286e7c973..7ef3947b7 100644 --- a/node/src/actors/chain_manager/mining.rs +++ b/node/src/actors/chain_manager/mining.rs @@ -1269,6 +1269,7 @@ pub fn build_block( if protocol_version > V1_8 { let mut included_validators = HashSet::::new(); + let mut understake_transactions = Vec::::new(); let max_ut_weight = consensus_constants_wit2.get_maximum_unstake_block_weight(epoch); for ut_tx in transactions_pool.ut_iter() { let validator_pkh = ut_tx.body.operator; @@ -1280,6 +1281,43 @@ pub fn build_block( continue; } + // If a set of unstaking transactions is sent simultaneously to the transactions pool using an amount which leaves + // more than 'min_stake' staked they can all be accepted since they do not introduce understaking yet. However, + // accepting all of them in subsequent blocks could violate the 'min_stake' rule. Thus we still need to check that + // we do not include all these unstaking transactions in a block so we do not produce an invalid block. + let stakes_key = QueryStakesKey::Key(StakeKey { + validator: ut_tx.body.operator, + withdrawer: ut_tx.body.withdrawal.pkh, + }); + let min_stake = consensus_constants_wit2.get_validator_min_stake_nanowits(epoch); + match stakes.query_stakes(stakes_key) { + Ok(stake_entry) => { + // TODO: modify this to enable delegated staking with multiple withdrawer addresses on a single validator + let staked_amount: u64 = stake_entry + .first() + .map(|stake| stake.value.coins) + .unwrap() + .into(); + if staked_amount - ut_tx.body.withdrawal.value - ut_tx.body.fee < min_stake { + log::info!("Unstaking with {} would result in understaking", { + ut_tx.hash() + }); + understake_transactions.push(ut_tx.hash()); + continue; + } + } + Err(_) => { + // Not finding a stake entry is possible if there are two concurrent unstake transactions where at least + // one of them unstakes all balance before the second one is included in a block. In that case, remove + // the latter one from our transaction pool. + log::info!("Cannot process unstake transaction {} since the full balance was already unstaked", { + ut_tx.hash() + }); + understake_transactions.push(ut_tx.hash()); + continue; + } + }; + let transaction_weight = ut_tx.weight(); let new_ut_weight = ut_weight.saturating_add(transaction_weight); if new_ut_weight <= max_ut_weight { @@ -1305,6 +1343,12 @@ pub fn build_block( included_validators.insert(validator_pkh); } + + transactions_pool.remove_understake_transactions(understake_transactions); + log::info!( + "There are {} unstake transactions in the transactions pool", + transactions_pool.ut_len() + ); } else { transactions_pool.clear_unstake_transactions(); } From 424518fb7f0cc0a3b9450b8ca6e7229da52ffb56 Mon Sep 17 00:00:00 2001 From: drcpu Date: Tue, 28 Jan 2025 13:00:59 +0000 Subject: [PATCH 10/13] feat(validations): add check to prevent multiple unstakes from the same operator in the same block --- data_structures/src/error.rs | 6 ++++++ validations/src/validations.rs | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/data_structures/src/error.rs b/data_structures/src/error.rs index 918a77191..45bffd670 100644 --- a/data_structures/src/error.rs +++ b/data_structures/src/error.rs @@ -515,6 +515,12 @@ pub enum BlockError { pkh )] RepeatedStakeOperator { pkh: PublicKeyHash }, + /// Repeated operator Unstake + #[fail( + display = "A single operator is withdrawing stake more than once in a block: ({}) ", + pkh + )] + RepeatedUnstakeOperator { pkh: PublicKeyHash }, /// Missing expected tallies #[fail( display = "{} expected tally transactions are missing in block candidate {}", diff --git a/validations/src/validations.rs b/validations/src/validations.rs index 2ce10a4b7..f9647f0f0 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -2406,6 +2406,19 @@ pub fn validate_block_transactions( let mut ut_mt = ProgressiveMerkleTree::sha256(); let mut ut_weight: u32 = 0; + // Check if the block contains more than one unstake tx from the same operator + let duplicate = block + .txns + .unstake_txns + .iter() + .map(|unstake_txn| unstake_txn.body.operator) + .duplicates() + .next(); + + if let Some(duplicate) = duplicate { + return Err(BlockError::RepeatedUnstakeOperator { pkh: duplicate }.into()); + } + for transaction in &block.txns.unstake_txns { let min_stake = consensus_constants_wit2.get_validator_min_stake_nanowits(epoch); let unstake_delay = consensus_constants_wit2.get_unstaking_delay_seconds(epoch); From 9ac1fec7b2e15e7395d4f74f67b73a3dfa3eb51c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ad=C3=A1n=20SDPC?= Date: Tue, 28 Jan 2025 14:28:54 +0100 Subject: [PATCH 11/13] feat(staking): set nonce to block epoch when needed This deprecates the old behavior to where nonces where purely sequential, and instead adopts a monotonic non-sequential system based on the epoch of the block that performed the latest user- initiated action on the stake entry, i.e. the most recent stake or unstake transaction. --- data_structures/src/staking/helpers.rs | 16 ++++++++ data_structures/src/staking/mod.rs | 2 +- data_structures/src/staking/stake.rs | 21 ++++++---- data_structures/src/staking/stakes.rs | 57 +++++++++++++++++--------- node/src/actors/chain_manager/mod.rs | 11 ++++- validations/src/validations.rs | 2 +- 6 files changed, 78 insertions(+), 31 deletions(-) diff --git a/data_structures/src/staking/helpers.rs b/data_structures/src/staking/helpers.rs index 8ffb5e924..698edaff4 100644 --- a/data_structures/src/staking/helpers.rs +++ b/data_structures/src/staking/helpers.rs @@ -499,6 +499,22 @@ where } } +/// Tells the stakes tracker what to do with the nonce associated to the entry or entries being +/// updated. +/// +/// This allows customizing the behavior of the nonce to be different when updating a stake entry +/// when processing a stake or unstake transaction vs. when adding rewards or enforcing slashing. +/// +/// Generally speaking, we want to update the nonce when we are processing a stake or unstake +/// transaction, but we want to keep the nonce the same if it is a reward or slashing act. +#[derive(Debug, PartialEq)] +pub enum NoncePolicy { + /// Update the value of the nonce field by deriving it from this epoch. + SetFromEpoch(Epoch), + /// Leave the value of the nonce field as is. + KeepAsIs, +} + #[cfg(test)] mod test { use super::*; diff --git a/data_structures/src/staking/mod.rs b/data_structures/src/staking/mod.rs index 57d300358..351a4e411 100644 --- a/data_structures/src/staking/mod.rs +++ b/data_structures/src/staking/mod.rs @@ -101,7 +101,7 @@ pub mod test { // If Alpha removes all of its stake, it should immediately disappear stakes - .remove_stake("Alpha", 2, true, MIN_STAKE_NANOWITS) + .remove_stake("Alpha", 2, 52, true, MIN_STAKE_NANOWITS) .unwrap(); let rank = stakes.by_rank(Capability::Mining, 51).collect::>(); assert_eq!( diff --git a/data_structures/src/staking/stake.rs b/data_structures/src/staking/stake.rs index dbea9c93d..ab5b96a83 100644 --- a/data_structures/src/staking/stake.rs +++ b/data_structures/src/staking/stake.rs @@ -22,7 +22,9 @@ where pub coins: Coins, /// The average epoch used to derive coin age for different capabilities. pub epochs: CapabilityMap, - /// The amount of stake and unstake actions. + /// A versioning number that gets updated upon unstaking, to guarantee resistance to replay + /// attacks and other potential issues that may arise from the lack of inputs in unstake + /// transactions. pub nonce: Nonce, /// This phantom field is here just for the sake of specifying generics. #[serde(skip)] @@ -65,6 +67,7 @@ where + Default + num_traits::Saturating + AddAssign + + From + From + Debug + Display @@ -78,6 +81,7 @@ where /// When adding stake: /// - Amounts are added together. /// - Epochs are weight-averaged, using the amounts as the weight. + /// - Nonces are overwritten. /// /// This type of averaging makes the entry equivalent to an unbounded record of all stake /// additions and removals, without the overhead in memory and computation. @@ -85,7 +89,7 @@ where &mut self, coins: Coins, epoch: Epoch, - increment_nonce: bool, + nonce_policy: NoncePolicy, minimum_stakeable: Coins, ) -> StakesResult { // Make sure that the amount to be staked is equal or greater than the minimum @@ -114,8 +118,11 @@ where self.epochs.update(capability, epoch_after); } - if increment_nonce { - self.nonce += Nonce::from(1); + // Nonces are updated following the "keep the latest epoch where this stake was updated + // manually" logic, where "manually" means by means of staking or unstaking, but not through + // rewards nor slashing. + if let NoncePolicy::SetFromEpoch(epoch) = nonce_policy { + self.nonce = Nonce::from(epoch); } Ok(coins_after) @@ -145,7 +152,7 @@ where pub fn remove_stake( &mut self, coins: Coins, - increment_nonce: bool, + nonce_policy: NoncePolicy, minimum_stakeable: Coins, ) -> StakesResult { let coins_after = self.coins.sub(coins); @@ -159,8 +166,8 @@ where self.coins = coins_after; - if increment_nonce { - self.nonce += Nonce::from(1); + if let NoncePolicy::SetFromEpoch(epoch) = nonce_policy { + self.nonce = Nonce::from(epoch); } Ok(self.coins) diff --git a/data_structures/src/staking/stakes.rs b/data_structures/src/staking/stakes.rs index befdf8932..fbf617982 100644 --- a/data_structures/src/staking/stakes.rs +++ b/data_structures/src/staking/stakes.rs @@ -216,7 +216,7 @@ where key: ISK, coins: Coins, epoch: Epoch, - increment_nonce: bool, + set_nonce: bool, minimum_stakeable: Coins, ) -> StakesResult, Address, Coins, Epoch> where @@ -242,11 +242,17 @@ where stake.key.write()?.withdrawer = key.withdrawer.clone(); } + let nonce_policy = if set_nonce { + NoncePolicy::SetFromEpoch(epoch) + } else { + NoncePolicy::KeepAsIs + }; + // Actually increase the number of coins stake .value .write()? - .add_stake(coins, epoch, increment_nonce, minimum_stakeable)?; + .add_stake(coins, epoch, nonce_policy, minimum_stakeable)?; // Update all indexes if needed (only when the stake entry didn't exist before) if !stake_found { @@ -374,7 +380,8 @@ where &mut self, key: ISK, coins: Coins, - increment_nonce: bool, + epoch: Epoch, + set_nonce: bool, minimum_stakeable: Coins, ) -> StakesResult where @@ -386,8 +393,13 @@ where // Reduce the amount of stake let final_coins = { let mut stake = by_address_entry.get_mut().value.write()?; + let nonce_policy = if set_nonce { + NoncePolicy::SetFromEpoch(epoch) + } else { + NoncePolicy::KeepAsIs + }; - stake.remove_stake(coins, increment_nonce, minimum_stakeable)? + stake.remove_stake(coins, nonce_policy, minimum_stakeable)? }; // No need to keep the entry if the stake has gone to zero @@ -524,11 +536,12 @@ where .ok_or(StakesError::ValidatorNotFound { validator })?; // TODO: modify this to enable delegated staking with multiple withdrawer addresses on a single validator - let _ = stakes[0] - .value - .write() - .unwrap() - .add_stake(coins, current_epoch, false, 0.into()); + let _ = stakes[0].value.write().unwrap().add_stake( + coins, + current_epoch, + NoncePolicy::KeepAsIs, + 0.into(), + ); Ok(()) } @@ -551,11 +564,11 @@ where .ok_or(StakesError::ValidatorNotFound { validator })?; // TODO: modify this to enable delegated staking with multiple withdrawer addresses on a single validator - let _ = stakes[0] - .value - .write() - .unwrap() - .remove_stake(coins, false, minimum_stakeable); + let _ = stakes[0].value.write().unwrap().remove_stake( + coins, + NoncePolicy::KeepAsIs, + minimum_stakeable, + ); Ok(()) } @@ -815,6 +828,7 @@ where key.validator.bech32(environment) ); + // When adding stake, epochs get averaged but nonces get overwritten stakes.add_stake(key, coins, epoch, true, minimum_stakeable.into())?; log::debug!("Current state of the stakes tracker: {:#?}", stakes); @@ -829,6 +843,7 @@ where pub fn process_unstake_transaction( stakes: &mut Stakes, transaction: &UnstakeTransaction, + epoch: Epoch, minimum_stakeable: u64, ) -> StakesResult<(), PublicKeyHash, Wit, Epoch> where @@ -875,7 +890,8 @@ where coins.wits_and_nanowits().0, ); - stakes.remove_stake(key, coins, true, minimum_stakeable.into())?; + // When removing stake, epochs get averaged but nonces get overwritten + stakes.remove_stake(key, coins, epoch, true, minimum_stakeable.into())?; log::debug!("Current state of the stakes tracker: {:#?}", stakes); @@ -935,6 +951,7 @@ where pub fn process_unstake_transactions<'a, const UNIT: u8, Epoch, Nonce, Power>( stakes: &mut Stakes, transactions: impl Iterator, + epoch: Epoch, minimum_stakeable: u64, ) -> Result<(), StakesError> where @@ -968,7 +985,7 @@ where u64: From + From, { for transaction in transactions { - process_unstake_transaction(stakes, transaction, minimum_stakeable)?; + process_unstake_transaction(stakes, transaction, epoch, minimum_stakeable)?; } Ok(()) @@ -1498,7 +1515,7 @@ mod tests { assert_eq!(stakes.query_nonce(bob_david), Ok(1)); stakes - .remove_stake(bob_david, 10, true, MIN_STAKE_NANOWITS) + .remove_stake(bob_david, 10, 11, true, MIN_STAKE_NANOWITS) .unwrap(); assert_eq!(stakes.query_nonce(alice_charlie), Ok(1)); assert_eq!(stakes.query_nonce(bob_david), Ok(2)); @@ -1511,7 +1528,7 @@ mod tests { // Test nonces not increasing stakes - .remove_stake(bob_david, 10, false, MIN_STAKE_NANOWITS) + .remove_stake(bob_david, 10, 31, false, MIN_STAKE_NANOWITS) .unwrap(); assert_eq!(stakes.query_nonce(alice_charlie), Ok(1)); assert_eq!(stakes.query_nonce(bob_david), Ok(3)); @@ -1704,14 +1721,14 @@ mod tests { .unwrap(); assert_eq!(stakes.query_nonce(bob_alice), Ok(1)); stakes - .remove_stake(bob_alice, 20, true, MIN_STAKE_NANOWITS) + .remove_stake(bob_alice, 20, 0, true, MIN_STAKE_NANOWITS) .unwrap(); stakes .add_stake(bob_alice, 20, 1, true, MIN_STAKE_NANOWITS) .unwrap(); assert_eq!(stakes.query_nonce(bob_alice), Ok(2)); stakes - .remove_stake(bob_alice, 20, true, MIN_STAKE_NANOWITS) + .remove_stake(bob_alice, 20, 1, true, MIN_STAKE_NANOWITS) .unwrap(); stakes .add_stake(bob_alice, 20, 2, true, MIN_STAKE_NANOWITS) diff --git a/node/src/actors/chain_manager/mod.rs b/node/src/actors/chain_manager/mod.rs index 85c098e4d..6639ed4c7 100644 --- a/node/src/actors/chain_manager/mod.rs +++ b/node/src/actors/chain_manager/mod.rs @@ -1117,6 +1117,7 @@ impl ChainManager { let _ = process_unstake_transactions( stakes, block.txns.unstake_txns.iter(), + block_epoch, minimum_stakeable, ); } @@ -5236,6 +5237,7 @@ mod tests { process_unstake_transactions( &mut stakes, vec![unstake_txn_1.clone()].iter(), + block_epoch, 10_000_000_000_000, ) .unwrap(); @@ -5297,8 +5299,13 @@ mod tests { assert_ne!(unstake_txn_1.hash(), unstake_txn_2.hash()); // Unstake all again for validator 2 - process_unstake_transactions(&mut stakes, vec![unstake_txn_2].iter(), 10_000_000_000_000) - .unwrap(); + process_unstake_transactions( + &mut stakes, + vec![unstake_txn_2].iter(), + block_epoch, + 10_000_000_000_000, + ) + .unwrap(); // Check validator 2 is removed from stakes again assert_eq!( diff --git a/validations/src/validations.rs b/validations/src/validations.rs index f9647f0f0..c355efe33 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -1459,7 +1459,7 @@ pub fn validate_stake_transaction<'a>( )) } -/// Function to validate a unstake transaction +/// Function to validate an unstake transaction pub fn validate_unstake_transaction<'a>( ut_tx: &'a UnstakeTransaction, epoch: Epoch, From 3a68ae45528949ed1a8a5b934139af2db544e5cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ad=C3=A1n=20SDPC?= Date: Tue, 28 Jan 2025 14:51:44 +0100 Subject: [PATCH 12/13] ref(validations): rework unstake value validation for clarity --- validations/src/validations.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/validations/src/validations.rs b/validations/src/validations.rs index c355efe33..44037cb82 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -1525,10 +1525,10 @@ pub fn validate_unstake_transaction<'a>( // Allowed unstake actions: // 1) Unstake the full balance (checked by the first condition) - // 2) Unstake an amount such that the leftover staked amount is greater than the min allowed - if staked_amount - amount_to_unstake > 0 - && staked_amount - amount_to_unstake < min_stake_nanowits - { + // 2) Unstake an amount such that the remainder (the stake amount that is left over) is greater + // than the minimum allowed + let remainder = staked_amount - amount_to_unstake; + if remainder > 0 && remainder < min_stake_nanowits { return Err(TransactionError::StakeBelowMinimum { min_stake: min_stake_nanowits, stake: staked_amount, From 00e750769cd2880e0aa34b9a02e333959a43cd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ad=C3=A1n=20SDPC?= Date: Tue, 28 Jan 2025 18:09:56 +0100 Subject: [PATCH 13/13] feat(data_requests): increase max witnesses on testnet and dev envs Originally by @guidiaz in b4a73d6d617c49206a1a50dd331f16d22c25f396 with edits from @drcpu-github --- node/src/actors/chain_manager/mod.rs | 4 ++-- validations/src/validations.rs | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/node/src/actors/chain_manager/mod.rs b/node/src/actors/chain_manager/mod.rs index 6639ed4c7..04ca16ee1 100644 --- a/node/src/actors/chain_manager/mod.rs +++ b/node/src/actors/chain_manager/mod.rs @@ -5236,7 +5236,7 @@ mod tests { // Unstake all for validator 2 process_unstake_transactions( &mut stakes, - vec![unstake_txn_1.clone()].iter(), + [unstake_txn_1.clone()].iter(), block_epoch, 10_000_000_000_000, ) @@ -5301,7 +5301,7 @@ mod tests { // Unstake all again for validator 2 process_unstake_transactions( &mut stakes, - vec![unstake_txn_2].iter(), + [unstake_txn_2].iter(), block_epoch, 10_000_000_000_000, ) diff --git a/validations/src/validations.rs b/validations/src/validations.rs index 44037cb82..7cd2413aa 100644 --- a/validations/src/validations.rs +++ b/validations/src/validations.rs @@ -1908,8 +1908,6 @@ pub fn update_utxo_diff<'a, IterInputs, IterOutputs>( output.clone() }; - log::info!("Inserting output {:?}", output_to_insert); - utxo_diff.insert_utxo(output_pointer, output_to_insert, block_number); } }