Skip to content

Commit

Permalink
Refactoring usage of Signer to ValidatorSigner (#2026)
Browse files Browse the repository at this point in the history
* Move Blake2b hashes to a separate module.

* Sort crypto dependencies.

* Universal ToScalar trait.

* WIP

* Revert "Proof-of-stake-time (#1848)"

This reverts commit 2c7bf9c.

* Removing weight, replacing fork choice with score and height

In preparation for implementing Doomslug, removing the concept of
`weight` and replacing the fork choice rule to choose the chain with the
highest score, and then height. Replacing score with the height of the
last pre-voted block rather than the weight.

* Doomslug the Destroyer

Implementation and integration of Doomslug (see https://near.ai/doomslug).

Doomslug itself is in `chain/doomslug.rs`, and doesn't depend on chain or any
other code. It also always takes the current time as an argument. It
significantly simplifies testing.

Doomslug is stored on the client, not chain, because both block
production and approvals processing happens on client. Instantiating on
chain would require a slightly more complex interface (e.g. it would be
impossible to pass `me`).

Doomslug needs to know about the latest Tip. Instead of intercepting the
lowest level tip-updating routine (which is in storage), I update the
Tip in the client when we accept the new head. It could miss head
changes related to syncing and challenges. To be safe I always update
the head whenever I interact with Doomslug, so the head is guaranteed to
be accurate whenever we do anything related to Doomslug, but it might
get sent to Doomslug with a slight delay.

This change also solves a problem that approvals before were in a cache
hash map, therefore an adversary could have spammed a node with lots of
invalid approvals and remove all valid approvals from the cache,
resulting in a node not having enough approvals when producing
the block. New logic is more complex, see
`DoomslugApprovalsTrackersAtHeight` class.

Test plan
---------
1. Sanity tests (basic invariants for `set_tip` and approval processing)
2. A fuzzy test that tests safety and liveness under different times to
GST and deltas.
3. `test_catchup_sanity_blocks_produced_doomslug` verifies that heights
are properly skipped.
4. Also added checking doomslug invariant into `cross_shard_tx`, which
is known to evoke all kinds of weird structures (but `cross_shard_tx`
operates without the requirement to have 50% approvals on the block,
thus still causing lots of forkfullness).
5. A new version of `cross_shard_tx` that enables doomslug, but disables
tampering with the finality gadget. Thus the vanilla version tests heavy
forkfulness and tampers with FG, while the new doomslug version has
practically no forfulness due to doomslug, and doesn't stress the FG as
much, but does test block production with doomslug.

* More WIP, some tests and fixes.

* Better names.

* Starting to add ValidatorSigner to abstract signage of different parts for validators

* Adding ristretto curve to the ValidatorSigner as additional curve

* Fix up warnings and config saving

* Correct ristretto group from seed creation

* Fix comment and failed test

* Optimize imports

* Fix doomslug tests

* Sorted imports

* Undo randomness branch here

* Undo undo vrf

* Addressing comments: adding telemetry types for clean API around signing and passing that information

Co-authored-by: Evgeny Kapun <[email protected]>
Co-authored-by: Alexander Skidanov <[email protected]>
  • Loading branch information
3 people authored Feb 5, 2020
1 parent 05a82d8 commit 9dd441c
Show file tree
Hide file tree
Showing 61 changed files with 993 additions and 696 deletions.
95 changes: 53 additions & 42 deletions chain/chain/src/doomslug.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use near_crypto::Signer;
use near_primitives::block::Approval;
use near_primitives::hash::CryptoHash;
use near_primitives::types::{AccountId, Balance, BlockHeight, BlockHeightDelta, ValidatorStake};
use std::collections::HashMap;
use std::convert::TryFrom;
use std::sync::Arc;
use std::time::{Duration, Instant};

use near_primitives::block::Approval;
use near_primitives::hash::CryptoHash;
use near_primitives::types::{AccountId, Balance, BlockHeight, BlockHeightDelta, ValidatorStake};
use near_primitives::validator_signer::ValidatorSigner;

/// Have that many iterations in the timer instead of `loop` to prevent potential bugs from blocking
/// the node
const MAX_TIMER_ITERS: usize = 20;
Expand Down Expand Up @@ -94,7 +95,6 @@ struct DoomslugApprovalsTrackersAtHeight {
/// happens via `PersistentDoomslug` struct. The split is to simplify testing of the logic separate
/// from the chain.
pub struct Doomslug {
me: Option<AccountId>,
approval_tracking: HashMap<BlockHeight, DoomslugApprovalsTrackersAtHeight>,
/// Largest height that we promised to skip
largest_promised_skip_height: BlockHeight,
Expand All @@ -110,7 +110,7 @@ pub struct Doomslug {
endorsement_pending: bool,
/// Information to track the timer (see `start_timer` routine in the paper)
timer: DoomslugTimer,
signer: Option<Arc<dyn Signer>>,
signer: Option<Arc<dyn ValidatorSigner>>,
/// How many approvals to have before producing a block. In production should be always `HalfStake`,
/// but for many tests we use `NoApprovals` to invoke more forkfulness
threshold_mode: DoomslugThresholdMode,
Expand Down Expand Up @@ -300,19 +300,16 @@ impl DoomslugApprovalsTrackersAtHeight {

impl Doomslug {
pub fn new(
me: Option<AccountId>,
largest_previously_skipped_height: BlockHeight,
largest_previously_endorsed_height: BlockHeight,
endorsement_delay: Duration,
min_delay: Duration,
delay_step: Duration,
max_delay: Duration,
signer: Option<Arc<dyn Signer>>,
signer: Option<Arc<dyn ValidatorSigner>>,
threshold_mode: DoomslugThresholdMode,
) -> Self {
assert_eq!(me.is_some(), signer.is_some());
Doomslug {
me,
approval_tracking: HashMap::new(),
largest_promised_skip_height: largest_previously_skipped_height,
largest_endorsed_height: largest_previously_endorsed_height,
Expand Down Expand Up @@ -442,14 +439,13 @@ impl Doomslug {
target_height: BlockHeight,
is_endorsement: bool,
) -> Option<Approval> {
self.me.as_ref().map(|me| {
self.signer.as_ref().map(|signer| {
Approval::new(
self.tip.block_hash,
self.tip.reference_hash,
target_height,
is_endorsement,
&**self.signer.as_ref().unwrap(),
me.clone(),
&**signer,
)
})
}
Expand Down Expand Up @@ -694,30 +690,32 @@ impl Doomslug {

#[cfg(test)]
mod tests {
use std::sync::Arc;
use std::time::{Duration, Instant};

use near_crypto::{KeyType, SecretKey};
use near_primitives::block::Approval;
use near_primitives::hash::hash;
use near_primitives::types::ValidatorStake;
use near_primitives::validator_signer::InMemoryValidatorSigner;

use crate::doomslug::{
DoomslugApprovalsTrackersAtHeight, DoomslugBlockProductionReadiness, DoomslugThresholdMode,
};
use crate::Doomslug;
use near_crypto::{InMemorySigner, KeyType, SecretKey};
use near_primitives::block::Approval;
use near_primitives::hash::hash;
use near_primitives::types::ValidatorStake;
use std::sync::Arc;
use std::time::{Duration, Instant};

#[test]
fn test_endorsements_and_skips_basic() {
let mut now = Instant::now(); // For the test purposes the absolute value of the initial instant doesn't matter

let mut ds = Doomslug::new(
Some("test".to_string()),
0,
0,
Duration::from_millis(400),
Duration::from_millis(1000),
Duration::from_millis(100),
Duration::from_millis(3000),
Some(Arc::new(InMemorySigner::from_seed("test", KeyType::ED25519, "test"))),
Some(Arc::new(InMemoryValidatorSigner::from_seed("test", KeyType::ED25519, "test"))),
DoomslugThresholdMode::HalfStake,
);

Expand Down Expand Up @@ -862,18 +860,25 @@ mod tests {

#[test]
fn test_doomslug_approvals() {
let stakes = vec![("test1", 2), ("test2", 1), ("test3", 3), ("test4", 2)]
.into_iter()
let accounts: Vec<(&str, u128)> =
vec![("test1", 2), ("test2", 1), ("test3", 3), ("test4", 2)];
let stakes = accounts
.iter()
.map(|(account_id, stake)| ValidatorStake {
account_id: account_id.to_string(),
stake,
stake: *stake,
public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
})
.collect::<Vec<_>>();
let signers = accounts
.iter()
.map(|(account_id, _)| {
InMemoryValidatorSigner::from_seed(account_id, KeyType::ED25519, account_id)
})
.collect::<Vec<_>>();

let signer = Arc::new(InMemorySigner::from_seed("test", KeyType::ED25519, "test"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed("test", KeyType::ED25519, "test"));
let mut ds = Doomslug::new(
Some("test".to_string()),
0,
0,
Duration::from_millis(400),
Expand All @@ -894,7 +899,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[1]), None, 2, true, &*signer, "test1".to_string()),
&Approval::new(hash(&[1]), None, 2, true, &signers[0]),
&stakes,
),
DoomslugBlockProductionReadiness::None,
Expand All @@ -904,7 +909,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[1]), None, 4, true, &*signer, "test3".to_string()),
&Approval::new(hash(&[1]), None, 4, true, &signers[2]),
&stakes,
),
DoomslugBlockProductionReadiness::None,
Expand All @@ -914,7 +919,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[1]), None, 4, true, &*signer, "test1".to_string()),
&Approval::new(hash(&[1]), None, 4, true, &signers[0]),
&stakes,
),
DoomslugBlockProductionReadiness::PassedThreshold(now),
Expand All @@ -924,7 +929,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now + Duration::from_millis(100),
&Approval::new(hash(&[1]), None, 4, true, &*signer, "test1".to_string()),
&Approval::new(hash(&[1]), None, 4, true, &signers[0]),
&stakes,
),
DoomslugBlockProductionReadiness::PassedThreshold(now),
Expand All @@ -934,7 +939,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[1]), None, 4, true, &*signer, "test4".to_string()),
&Approval::new(hash(&[1]), None, 4, true, &signers[3]),
&stakes,
),
DoomslugBlockProductionReadiness::ReadyToProduce(now),
Expand All @@ -944,7 +949,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[1]), None, 2, true, &*signer, "test4".to_string()),
&Approval::new(hash(&[1]), None, 2, true, &signers[3]),
&stakes,
),
DoomslugBlockProductionReadiness::None,
Expand All @@ -956,7 +961,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[1]), None, 2, true, &*signer, "test2".to_string()),
&Approval::new(hash(&[1]), None, 2, true, &signers[1]),
&stakes,
),
DoomslugBlockProductionReadiness::PassedThreshold(now),
Expand All @@ -966,7 +971,7 @@ mod tests {
assert_eq!(
ds.on_approval_message_internal(
now,
&Approval::new(hash(&[2]), None, 4, true, &*signer, "test2".to_string()),
&Approval::new(hash(&[2]), None, 4, true, &signers[1]),
&stakes,
),
DoomslugBlockProductionReadiness::None,
Expand All @@ -975,24 +980,30 @@ mod tests {

#[test]
fn test_doomslug_one_approval_per_target_height() {
let stakes = vec![("test1", 2), ("test2", 1), ("test3", 3), ("test4", 2)]
let accounts = vec![("test1", 2), ("test2", 1), ("test3", 3), ("test4", 2)];
let signers = accounts
.iter()
.map(|(account_id, _)| {
InMemoryValidatorSigner::from_seed(account_id, KeyType::ED25519, account_id)
})
.collect::<Vec<_>>();
let stakes = accounts
.into_iter()
.map(|(account_id, stake)| ValidatorStake {
account_id: account_id.to_string(),
stake,
public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
})
.collect::<Vec<_>>();
let signer = Arc::new(InMemorySigner::from_seed("test", KeyType::ED25519, "test"));
let mut tracker = DoomslugApprovalsTrackersAtHeight::new();

let a1_1 = Approval::new(hash(&[1]), None, 4, true, &*signer, "test1".to_string());
let a1_2 = Approval::new(hash(&[1]), None, 4, false, &*signer, "test2".to_string());
let a1_3 = Approval::new(hash(&[1]), None, 4, true, &*signer, "test3".to_string());
let a1_1 = Approval::new(hash(&[1]), None, 4, true, &signers[0]);
let a1_2 = Approval::new(hash(&[1]), None, 4, false, &signers[1]);
let a1_3 = Approval::new(hash(&[1]), None, 4, true, &signers[2]);

let a2_1 = Approval::new(hash(&[2]), None, 4, true, &*signer, "test1".to_string());
let a2_2 = Approval::new(hash(&[2]), None, 4, false, &*signer, "test2".to_string());
let a2_3 = Approval::new(hash(&[2]), None, 4, true, &*signer, "test3".to_string());
let a2_1 = Approval::new(hash(&[2]), None, 4, true, &signers[0]);
let a2_2 = Approval::new(hash(&[2]), None, 4, false, &signers[1]);
let a2_3 = Approval::new(hash(&[2]), None, 4, true, &signers[2]);

// Process first approval, and then process it again and make sure it works
tracker.process_approval(Instant::now(), &a1_1, &stakes, DoomslugThresholdMode::HalfStake);
Expand Down
8 changes: 5 additions & 3 deletions chain/chain/src/finality.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::error::{Error, ErrorKind};
use crate::{ChainStoreAccess, ChainStoreUpdate};
use std::collections::{HashMap, HashSet};

use near_primitives::block::{
Approval, BlockHeader, BlockHeaderInnerLite, BlockHeaderInnerRest, BlockScore,
};
use near_primitives::hash::CryptoHash;
use near_primitives::types::{AccountId, Balance, BlockHeight, EpochId, ValidatorStake};
use std::collections::{HashMap, HashSet};

use crate::error::{Error, ErrorKind};
use crate::{ChainStoreAccess, ChainStoreUpdate};

// How many blocks back to search for a new reference hash when the chain switches and the block
// producer cannot use the same reference hash as the last approval on chain
Expand Down
8 changes: 5 additions & 3 deletions chain/chain/src/lightclient.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use crate::error::Error;
use crate::{ChainStoreAccess, ErrorKind, RuntimeAdapter};
use std::collections::{HashMap, HashSet};

use near_primitives::block::BlockHeader;
use near_primitives::hash::CryptoHash;
use near_primitives::types::EpochId;
use near_primitives::views::{
BlockHeaderInnerLiteView, LightClientApprovalView, LightClientBlockView, ValidatorStakeView,
};
use std::collections::{HashMap, HashSet};

use crate::error::Error;
use crate::{ChainStoreAccess, ErrorKind, RuntimeAdapter};

const MAX_LIGHT_CLIENT_FUTURE_BLOCKS: usize = 50;

Expand Down
Loading

0 comments on commit 9dd441c

Please sign in to comment.