Skip to content

Commit

Permalink
refactor: Reintroduce strictly typed AccountId (#4621)
Browse files Browse the repository at this point in the history
This PR introduces a strictly typed structure - `AccountId` that enforces account ID validation.
This way, we can ensure validity and thereby fail fast before an invalid account name is used.

This also, optionally, ensures validation on {,de}serialization so, yeah..

The structure is defined within its own self-contained crate `near-account-id` to allow for easy publishing.
The extra dependencies - `borsh`, `serde` are made optional to allow the crate to be lightweight enough to be depended upon.

Within `nearcore`, the structure is reexported via `near_primitives::AccountId` and the crate via `near_primitives{,_core}::account::id` with default features (`borsh`, `serde`) but you can opt-out of those by depending on it directly.

Resolves #2074, supersedes #2831.

* [x] Existing unit tests pass with minor updates
* [x] Nayduck tests pass without modifications
* [x] Manually ensured that the protocol is not changed

* [x] Manually booted a testnet node to ensure genesis loads properly and that no further AccountId-related issues occur.
* [x] Ensured unit tests pass with no further updates
* [x] Nayduck tests pass without modifications
  • Loading branch information
miraclx authored and nikurt committed Aug 9, 2021
1 parent 9a4d3ba commit 4eeb55d
Show file tree
Hide file tree
Showing 161 changed files with 4,096 additions and 3,018 deletions.
29 changes: 17 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ resolver = "2"
members = [
"core/chain-configs",
"core/crypto",
"core/account-id",
"core/primitives",
"core/primitives-core",
"core/store",
Expand Down
30 changes: 23 additions & 7 deletions chain/chain/src/doomslug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ mod tests {
use near_crypto::{KeyType, SecretKey};
use near_primitives::block::{Approval, ApprovalInner};
use near_primitives::hash::hash;
use near_primitives::types::ApprovalStake;
use near_primitives::types::{AccountId, ApprovalStake};
use near_primitives::validator_signer::InMemoryValidatorSigner;

use crate::doomslug::{
Expand All @@ -618,7 +618,11 @@ mod tests {
Duration::from_millis(1000),
Duration::from_millis(100),
Duration::from_millis(3000),
Some(Arc::new(InMemoryValidatorSigner::from_seed("test", KeyType::ED25519, "test"))),
Some(Arc::new(InMemoryValidatorSigner::from_seed(
AccountId::test_account(),
KeyType::ED25519,
"test",
))),
DoomslugThresholdMode::TwoThirds,
);

Expand Down Expand Up @@ -745,7 +749,7 @@ mod tests {
let stakes = accounts
.iter()
.map(|(account_id, stake_this_epoch, stake_next_epoch)| ApprovalStake {
account_id: account_id.to_string(),
account_id: account_id.parse().unwrap(),
stake_this_epoch: *stake_this_epoch,
stake_next_epoch: *stake_next_epoch,
public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
Expand All @@ -755,11 +759,19 @@ mod tests {
let signers = accounts
.iter()
.map(|(account_id, _, _)| {
InMemoryValidatorSigner::from_seed(account_id, KeyType::ED25519, account_id)
InMemoryValidatorSigner::from_seed(
account_id.parse().unwrap(),
KeyType::ED25519,
account_id,
)
})
.collect::<Vec<_>>();

let signer = Arc::new(InMemoryValidatorSigner::from_seed("test", KeyType::ED25519, "test"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
AccountId::test_account(),
KeyType::ED25519,
"test",
));
let mut ds = Doomslug::new(
0,
Duration::from_millis(400),
Expand Down Expand Up @@ -875,13 +887,17 @@ mod tests {
let signers = accounts
.iter()
.map(|(account_id, _, _)| {
InMemoryValidatorSigner::from_seed(account_id, KeyType::ED25519, account_id)
InMemoryValidatorSigner::from_seed(
account_id.parse().unwrap(),
KeyType::ED25519,
account_id,
)
})
.collect::<Vec<_>>();
let stakes = accounts
.into_iter()
.map(|(account_id, stake_this_epoch, stake_next_epoch)| ApprovalStake {
account_id: account_id.to_string(),
account_id: account_id.parse().unwrap(),
stake_this_epoch,
stake_next_epoch,
public_key: SecretKey::from_seed(KeyType::ED25519, account_id).public_key(),
Expand Down
57 changes: 41 additions & 16 deletions chain/chain/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2787,7 +2787,9 @@ impl<'a> ChainStoreUpdate<'a> {
}
}
for (account_id, approval) in last_approvals_per_account {
self.chain_store.last_approvals_per_account.cache_set(account_id.into(), approval);
self.chain_store
.last_approvals_per_account
.cache_set(account_id.as_ref().into(), approval);
}
for (block_hash, next_hash) in next_block_hashes {
self.chain_store.next_block_hashes.cache_set(block_hash.into(), next_hash);
Expand Down Expand Up @@ -2889,7 +2891,9 @@ mod tests {
store.clone(),
validators
.into_iter()
.map(|inner| inner.into_iter().map(Into::into).collect())
.map(|inner| {
inner.into_iter().map(|account_id| account_id.parse().unwrap()).collect()
})
.collect(),
1,
1,
Expand All @@ -2903,8 +2907,11 @@ mod tests {
let transaction_validity_period = 5;
let mut chain = get_chain();
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let short_fork = vec![Block::empty_with_height(&genesis, 1, &*signer.clone())];
let mut store_update = chain.mut_store().store_update();
store_update.save_block_header(short_fork[0].header().clone()).unwrap();
Expand Down Expand Up @@ -2958,8 +2965,11 @@ mod tests {
let transaction_validity_period = 5;
let mut chain = get_chain();
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let mut blocks = vec![];
let mut prev_block = genesis.clone();
for i in 1..(transaction_validity_period + 2) {
Expand Down Expand Up @@ -3009,8 +3019,11 @@ mod tests {
let transaction_validity_period = 5;
let mut chain = get_chain();
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let mut short_fork = vec![];
let mut prev_block = genesis.clone();
for i in 1..(transaction_validity_period + 2) {
Expand Down Expand Up @@ -3056,8 +3069,11 @@ mod tests {
fn test_cache_invalidation() {
let mut chain = get_chain();
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let block1 = Block::empty_with_height(&genesis, 1, &*signer.clone());
let mut block2 = block1.clone();
block2.mut_header().get_mut().inner_lite.epoch_id = EpochId(hash(&[1, 2, 3]));
Expand Down Expand Up @@ -3097,8 +3113,11 @@ mod tests {
fn test_clear_old_data() {
let mut chain = get_chain_with_epoch_length(1);
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let mut prev_block = genesis.clone();
let mut blocks = vec![prev_block.clone()];
for i in 1..15 {
Expand Down Expand Up @@ -3191,8 +3210,11 @@ mod tests {
fn test_clear_old_data_fixed_height() {
let mut chain = get_chain();
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let mut prev_block = genesis.clone();
let mut blocks = vec![prev_block.clone()];
for i in 1..10 {
Expand Down Expand Up @@ -3267,8 +3289,11 @@ mod tests {
fn test_clear_old_data_too_many_heights_common(gc_blocks_limit: NumBlocks) {
let mut chain = get_chain_with_epoch_length(1);
let genesis = chain.get_block_by_height(0).unwrap().clone();
let signer =
Arc::new(InMemoryValidatorSigner::from_seed("test1", KeyType::ED25519, "test1"));
let signer = Arc::new(InMemoryValidatorSigner::from_seed(
"test1".parse().unwrap(),
KeyType::ED25519,
"test1",
));
let mut prev_block = genesis.clone();
let mut blocks = vec![prev_block.clone()];
{
Expand Down
Loading

0 comments on commit 4eeb55d

Please sign in to comment.