Skip to content

Commit

Permalink
feat(tapi): also persist v1_8 in chain_info when activated
Browse files Browse the repository at this point in the history
  • Loading branch information
@drcpu-github
drcpu-github committed Nov 11, 2024
1 parent aa2670d commit 5b7e96d
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 37 deletions.
50 changes: 31 additions & 19 deletions data_structures/src/chain/tapi.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
chain::{Environment, Epoch, PublicKeyHash},
chain::{ChainInfo, Environment, Epoch, PublicKeyHash},
register_protocol_version, ProtocolVersion,
};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -139,6 +139,8 @@ impl TapiEngine {
epoch_to_update: Epoch,
block_epoch: Epoch,
avoid_wip_list: &HashSet<String>,
checkpoints_period: u16,
chain_info: &mut ChainInfo,
) {
// In case of empty epochs, they would be considered as blocks with tapi version to 0
// In order to not update bit counter from old blocks where the block version was not used,
Expand All @@ -149,7 +151,7 @@ impl TapiEngine {
let init = self.bit_tapi_counter.last_epoch + 1;
let end = epoch_to_update;
for i in init..end {
self.update_bit_counter(0, i, block_epoch, avoid_wip_list);
self.update_bit_counter(0, i, block_epoch, avoid_wip_list, checkpoints_period, chain_info);
}
}
for n in 0..self.bit_tapi_counter.len() {
Expand All @@ -170,7 +172,14 @@ impl TapiEngine {
register_protocol_version(
ProtocolVersion::V1_8,
block_epoch + 21,
45,
checkpoints_period,
);
// Register the 1_8 protocol into chain state (namely, chain info) so that
// the scheduled activation data eventually gets persisted into storage.
chain_info.protocol.register(
block_epoch + 11,
ProtocolVersion::V1_8,
checkpoints_period,
);
}
}
Expand Down Expand Up @@ -665,6 +674,7 @@ mod tests {
#[test]
fn test_update_bit_counter() {
let empty_hs = HashSet::default();
let mut chain_info = ChainInfo::default();
let mut t = TapiEngine::default();
let bit = 0;
let wip = BitVotesCounter {
Expand All @@ -678,42 +688,42 @@ mod tests {
t.bit_tapi_counter.insert(wip);
assert_eq!(t.bit_tapi_counter.last_epoch, 0);

t.update_bit_counter(1, 9_999, 9_999, &empty_hs);
t.update_bit_counter(1, 9_999, 9_999, &empty_hs, 45, &mut chain_info);
// Updating with epoch < init does not increase the votes counter
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 0);
assert_eq!(t.bit_tapi_counter.last_epoch, 9_999);

t.update_bit_counter(1, 10_000, 10_000, &empty_hs);
t.update_bit_counter(1, 10_000, 10_000, &empty_hs, 45, &mut chain_info);
// Updating with epoch >= init does increase the votes counter
// But since this is the first epoch, the votes counter is reset to 0 again afterwards
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 0);

t.update_bit_counter(1, 10_001, 10_001, &empty_hs);
t.update_bit_counter(1, 10_001, 10_001, &empty_hs, 45, &mut chain_info);
// Updating with epoch >= init does increase the votes counter
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 1);

t.update_bit_counter(1, 10_002, 10_002, &empty_hs);
t.update_bit_counter(1, 10_002, 10_002, &empty_hs, 45, &mut chain_info);
// Updating with epoch >= init does increase the votes counter
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 2);

// Updating with an epoch that was already updated will count the votes twice, there is no
// protection against this because the update_new_wip_votes function must be able to count
// votes from old blocks
/*
t.update_bit_counter(1, 10_002, &empty_hs);
t.update_bit_counter(1, 10_002, &empty_hs, 45, &mut chain_info);
*/

t.update_bit_counter(0, 10_003, 10_003, &empty_hs);
t.update_bit_counter(0, 10_003, 10_003, &empty_hs, 45, &mut chain_info);
// Updating with epoch >= init but voting against does not increase the votes counter
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 2);

t.update_bit_counter(0, 10_103, 10_103, &empty_hs);
t.update_bit_counter(0, 10_103, 10_103, &empty_hs, 45, &mut chain_info);
// The vote counting is at epoch 10_100, the votes should be reset to 0
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 0);

// Add 90 votes to test activation
for epoch in 10_200..10_290 {
t.update_bit_counter(1, epoch, epoch, &empty_hs);
t.update_bit_counter(1, epoch, epoch, &empty_hs, 45, &mut chain_info);
}
// More than 80% of votes means that the WIP should activate at the next counting epoch
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 89);
Expand All @@ -724,7 +734,7 @@ mod tests {
// so that all the blocks after 10_321 are validated using the new validation logic, or
// change the WIP activation date to 10_501?
// Decided to change the WIP activation date to 10_521
t.update_bit_counter(0, 10_500, 10_500, &empty_hs);
t.update_bit_counter(0, 10_500, 10_500, &empty_hs, 45, &mut chain_info);
// The votes counter should reset
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 0);
// The activation date should be
Expand All @@ -734,6 +744,7 @@ mod tests {
#[test]
fn test_update_bit_counter_multi_vote() {
let empty_hs = HashSet::default();
let mut chain_info = ChainInfo::default();
let mut t = TapiEngine::default();
let wip0 = BitVotesCounter {
votes: 0,
Expand All @@ -756,35 +767,35 @@ mod tests {
assert_eq!(t.bit_tapi_counter.last_epoch, 0);

// Vote for none
t.update_bit_counter(0, 10_001, 10_001, &empty_hs);
t.update_bit_counter(0, 10_001, 10_001, &empty_hs, 45, &mut chain_info);
assert_eq!(t.bit_tapi_counter.info[0].clone().unwrap().votes, 0);
assert_eq!(t.bit_tapi_counter.info[1].clone().unwrap().votes, 0);
assert_eq!(t.bit_tapi_counter.last_epoch, 10_001);

// Vote for both
t.update_bit_counter(3, 10_002, 10_002, &empty_hs);
t.update_bit_counter(3, 10_002, 10_002, &empty_hs, 45, &mut chain_info);
assert_eq!(t.bit_tapi_counter.info[0].clone().unwrap().votes, 1);
assert_eq!(t.bit_tapi_counter.info[1].clone().unwrap().votes, 1);

// Vote only for wip0
t.update_bit_counter(1, 10_002, 10_002, &empty_hs);
t.update_bit_counter(1, 10_002, 10_002, &empty_hs, 45, &mut chain_info);
assert_eq!(t.bit_tapi_counter.info[0].clone().unwrap().votes, 2);
assert_eq!(t.bit_tapi_counter.info[1].clone().unwrap().votes, 1);

// Vote only for wip1
t.update_bit_counter(2, 10_002, 10_002, &empty_hs);
t.update_bit_counter(2, 10_002, 10_002, &empty_hs, 45, &mut chain_info);
assert_eq!(t.bit_tapi_counter.info[0].clone().unwrap().votes, 2);
assert_eq!(t.bit_tapi_counter.info[1].clone().unwrap().votes, 2);

// Add 90 votes to test activation of both wips in the same epoch
for epoch in 10_003..10_093 {
t.update_bit_counter(3, epoch, epoch, &empty_hs);
t.update_bit_counter(3, epoch, epoch, &empty_hs, 45, &mut chain_info);
}

assert_eq!(t.bit_tapi_counter.info[0].clone().unwrap().votes, 92);
assert_eq!(t.bit_tapi_counter.info[1].clone().unwrap().votes, 92);

t.update_bit_counter(0, 10_100, 10_100, &empty_hs);
t.update_bit_counter(0, 10_100, 10_100, &empty_hs, 45, &mut chain_info);
// The votes counter should reset
assert_eq!(t.bit_tapi_counter.info[0].clone().unwrap().votes, 0);
assert_eq!(t.bit_tapi_counter.info[1].clone().unwrap().votes, 0);
Expand All @@ -798,6 +809,7 @@ mod tests {
// Check that voting for unallocated wips is allowed, but the extra votes are not counted,
// and the votes for active bits are valid
let empty_hs = HashSet::default();
let mut chain_info = ChainInfo::default();
let mut t = TapiEngine::default();
let bit = 0;
let wip = BitVotesCounter {
Expand All @@ -812,7 +824,7 @@ mod tests {
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 0);

// Vote "yes" to all the 32 bits, even though there is only 1 active wip (bit 0)
t.update_bit_counter(u32::MAX, 10_001, 10_001, &empty_hs);
t.update_bit_counter(u32::MAX, 10_001, 10_001, &empty_hs, 45, &mut chain_info);
// This is a valid block and a valid vote
assert_eq!(t.bit_tapi_counter.info[bit].clone().unwrap().votes, 1);
}
Expand Down
47 changes: 30 additions & 17 deletions node/src/actors/chain_manager/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,17 @@ impl ChainManager {
// Insert candidate block into `block_chain` state
self.chain_state.block_chain.insert(block_epoch, block_hash);

// Update votes counter for WIPs
let checkpoints_period = chain_info.consensus_constants.checkpoints_period;
self.chain_state.tapi_engine.update_bit_counter(
block_signals,
block_epoch,
block_epoch,
&HashSet::default(),
checkpoints_period,
chain_info,
);

match self.sm_state {
StateMachine::WaitingConsensus => {
// Persist finished data requests into storage
Expand Down Expand Up @@ -1420,14 +1431,6 @@ impl ChainManager {
// Update transaction priority information
self.priority_engine.push_priorities(priorities);

// Update votes counter for WIPs
self.chain_state.tapi_engine.update_bit_counter(
block_signals,
block_epoch,
block_epoch,
&HashSet::default(),
);

if miner_pkh == own_pkh {
self.chain_state.node_stats.block_mined_count += 1;
if self.sm_state == StateMachine::Synced {
Expand Down Expand Up @@ -2793,14 +2796,21 @@ impl ChainManager {
}
.into_actor(self)
.and_then(move |block_headers, act, _ctx| {
for block_header in block_headers {
act.chain_state.tapi_engine.update_bit_counter(
block_header.signals,
block_header.beacon.checkpoint,
block_header.beacon.checkpoint,
&old_wips,
);
}
if let Some(chain_info) = act.chain_state.chain_info.as_mut() {
let checkpoints_period = chain_info.consensus_constants.checkpoints_period;
for block_header in block_headers {
act.chain_state.tapi_engine.update_bit_counter(
block_header.signals,
block_header.beacon.checkpoint,
block_header.beacon.checkpoint,
&old_wips,
checkpoints_period,
chain_info,
);
}
} else {
log::error!("Could not get chain info");
};

actix::fut::ok(())
});
Expand Down Expand Up @@ -3733,7 +3743,7 @@ mod tests {
OutputPointer, PartialConsensusConstants, PublicKey, SecretKey, Signature,
ValueTransferOutput,
},
proto::versioning::VersionedHashable,
proto::versioning::{ProtocolInfo, VersionedHashable},
transaction::{
CommitTransaction, DRTransaction, MintTransaction, RevealTransaction, VTTransaction,
VTTransactionBody,
Expand Down Expand Up @@ -3882,6 +3892,7 @@ mod tests {
hash_prev_block: Hash::SHA256([1; 32]),
},
highest_vrf_output: CheckpointVRF::default(),
protocol: ProtocolInfo::default(),
});

assert_eq!(
Expand Down Expand Up @@ -4301,6 +4312,7 @@ mod tests {
hash_prev_block: Hash::SHA256([1; 32]),
},
highest_vrf_output: CheckpointVRF::default(),
protocol: ProtocolInfo::default(),
});
chain_manager.chain_state.reputation_engine = Some(ReputationEngine::new(1000));
chain_manager.vrf_ctx = Some(VrfCtx::secp256k1().unwrap());
Expand Down Expand Up @@ -4435,6 +4447,7 @@ mod tests {
hash_prev_block: Hash::SHA256([1; 32]),
},
highest_vrf_output: CheckpointVRF::default(),
protocol: ProtocolInfo::default(),
});
let out_ptr = OutputPointer {
transaction_id: "0000000000000000000000000000000000000000000000000000000000000001"
Expand Down
3 changes: 2 additions & 1 deletion node/tests/serialization.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use bincode::{deserialize, serialize};
use serde::{de::DeserializeOwned, Serialize};
use std::fmt::Debug;
use witnet_data_structures::chain::*;
use witnet_data_structures::{chain::*, proto::versioning::ProtocolInfo};

fn t<T>(al: T)
where
Expand Down Expand Up @@ -57,6 +57,7 @@ fn chain_state() {
checkpoint: 0,
hash_prev_vrf: bootstrap_hash,
},
protocol: ProtocolInfo::default(),
};
let c = ChainState {
chain_info: Some(chain_info),
Expand Down

0 comments on commit 5b7e96d

Please sign in to comment.