Skip to content

Commit

Permalink
[feature] #4212: Prevent account registration without signatures
Browse files Browse the repository at this point in the history
Signed-off-by: Daniil Polyakov <[email protected]>
  • Loading branch information
Arjentix committed Feb 20, 2024
1 parent 64e9e31 commit 183ea07
Show file tree
Hide file tree
Showing 41 changed files with 182 additions and 109 deletions.
2 changes: 1 addition & 1 deletion cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ enum TelemetryStartStatus {
}

fn genesis_account(public_key: PublicKey) -> Account {
Account::new(iroha_genesis::GENESIS_ACCOUNT_ID.clone(), [public_key])
Account::new(iroha_genesis::GENESIS_ACCOUNT_ID.clone(), public_key)
.build(&iroha_genesis::GENESIS_ACCOUNT_ID)
}

Expand Down
4 changes: 2 additions & 2 deletions client/benches/torii.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn query_requests(criterion: &mut Criterion) {
let create_domain = Register::domain(Domain::new(domain_id.clone()));
let account_id = AccountId::new(domain_id.clone(), "account".parse().expect("Valid"));
let (public_key, _) = KeyPair::generate().into();
let create_account = Register::account(Account::new(account_id.clone(), [public_key]));
let create_account = Register::account(Account::new(account_id.clone(), public_key));
let asset_definition_id = AssetDefinitionId::new(domain_id, "xor".parse().expect("Valid"));
let create_asset =
Register::asset_definition(AssetDefinition::quantity(asset_definition_id.clone()));
Expand Down Expand Up @@ -164,7 +164,7 @@ fn instruction_submits(criterion: &mut Criterion) {
let create_domain: InstructionBox = Register::domain(Domain::new(domain_id.clone())).into();
let account_id = AccountId::new(domain_id.clone(), "account".parse().expect("Valid"));
let (public_key, _) = KeyPair::generate().into();
let create_account = Register::account(Account::new(account_id.clone(), [public_key])).into();
let create_account = Register::account(Account::new(account_id.clone(), public_key)).into();
let asset_definition_id = AssetDefinitionId::new(domain_id, "xor".parse().expect("Valid"));
let client_config = iroha_client::samples::get_client_config(
get_chain_id(),
Expand Down
3 changes: 1 addition & 2 deletions client/benches/tps/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ impl MeasurerUnit {
let account_id = account_id(self.name);
let asset_id = asset_id(self.name);

let register_me =
Register::account(Account::new(account_id, [keypair.public_key().clone()]));
let register_me = Register::account(Account::new(account_id, keypair.public_key().clone()));
self.client.submit_blocking(register_me)?;

let mint_a_rose = Mint::asset_quantity(1_u32, asset_id);
Expand Down
5 changes: 4 additions & 1 deletion client/examples/million_accounts_genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ fn create_million_accounts_directly() {
format!("bob-{i}").parse().expect("Valid"),
);
let create_domain: InstructionBox = Register::domain(Domain::new(domain_id)).into();
let create_account = Register::account(Account::new(normal_account_id.clone(), [])).into();
let create_account = Register::account(Account::new_with_random_signature(
normal_account_id.clone(),
))
.into();
if test_client
.submit_all([create_domain, create_account])
.is_err()
Expand Down
2 changes: 1 addition & 1 deletion client/examples/tutorial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn account_registration_test(config: Config) -> Result<(), Error> {

// #region register_account_generate
// Generate a new account
let create_account = Register::account(Account::new(account_id, [public_key]));
let create_account = Register::account(Account::new(account_id, public_key));
// #endregion register_account_generate

// #region register_account_prepare_tx
Expand Down
8 changes: 6 additions & 2 deletions client/tests/integration/add_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,18 @@ fn client_add_account_with_name_length_more_than_limit_should_not_commit_transac
let pipeline_time = Config::pipeline_time();

let normal_account_id: AccountId = "bob@wonderland".parse().expect("Valid");
let create_account = Register::account(Account::new(normal_account_id.clone(), []));
let create_account = Register::account(Account::new_with_random_signature(
normal_account_id.clone(),
));
test_client.submit(create_account)?;

let too_long_account_name = "0".repeat(2_usize.pow(14));
let incorrect_account_id: AccountId = (too_long_account_name + "@wonderland")
.parse()
.expect("Valid");
let create_account = Register::account(Account::new(incorrect_account_id.clone(), []));
let create_account = Register::account(Account::new_with_random_signature(
incorrect_account_id.clone(),
));
test_client.submit(create_account)?;

thread::sleep(pipeline_time * 2);
Expand Down
13 changes: 5 additions & 8 deletions client/tests/integration/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ fn find_rate_and_make_exchange_isi_should_succeed() {
let buyer_keypair = KeyPair::generate();

let register_account = |account_id: AccountId, signature: PublicKey| {
Register::account(Account::new(account_id, [signature]))
Register::account(Account::new(account_id, signature))
};

let grant_alice_asset_transfer_permission = |asset_id: AssetId, owner_keypair: KeyPair| {
Expand Down Expand Up @@ -454,13 +454,10 @@ mod register {
}

pub fn account(account_name: &str, domain_name: &str) -> Register<Account> {
Register::account(Account::new(
AccountId::new(
domain_name.parse().expect("Valid"),
account_name.parse().expect("Valid"),
),
[],
))
Register::account(Account::new_with_random_signature(AccountId::new(
domain_name.parse().expect("Valid"),
account_name.parse().expect("Valid"),
)))
}

pub fn asset_definition(asset_name: &str, domain_name: &str) -> Register<AssetDefinition> {
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/asset_propagation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn client_add_asset_quantity_to_existing_asset_should_increase_asset_amount_on_a
Register::domain(Domain::new(DomainId::from_str("domain")?)).into();
let account_id = AccountId::from_str("account@domain")?;
let (public_key, _) = KeyPair::generate().into();
let create_account = Register::account(Account::new(account_id.clone(), [public_key])).into();
let create_account = Register::account(Account::new(account_id.clone(), public_key)).into();
let asset_definition_id = AssetDefinitionId::from_str("xor#domain")?;
let create_asset =
Register::asset_definition(AssetDefinition::quantity(asset_definition_id.clone())).into();
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/burn_public_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn public_keys_cannot_be_burned_to_nothing() {
let charlie_initial_keypair = KeyPair::generate();
let register_charlie = Register::account(Account::new(
charlie_id.clone(),
[charlie_initial_keypair.public_key().clone()],
charlie_initial_keypair.public_key().clone(),
));

let (tx_hash, res) = submit(&client, [register_charlie], None);
Expand Down
14 changes: 7 additions & 7 deletions client/tests/integration/domain_owner_permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn domain_owner_domain_permissions() -> Result<()> {
test_client.submit_blocking(Register::domain(kingdom))?;

let bob_keypair = KeyPair::generate();
let bob = Account::new(bob_id.clone(), [bob_keypair.public_key().clone()]);
let bob = Account::new(bob_id.clone(), bob_keypair.public_key().clone());
test_client.submit_blocking(Register::account(bob))?;

// Asset definitions can't be registered by "bob@kingdom" by default
Expand Down Expand Up @@ -96,7 +96,7 @@ fn domain_owner_account_permissions() -> Result<()> {
let mad_hatter_keypair = KeyPair::generate();
let mad_hatter = Account::new(
mad_hatter_id.clone(),
[mad_hatter_keypair.public_key().clone()],
mad_hatter_keypair.public_key().clone(),
);
test_client.submit_blocking(Register::account(mad_hatter))?;

Expand Down Expand Up @@ -158,10 +158,10 @@ fn domain_owner_asset_definition_permissions() -> Result<()> {
test_client.submit_blocking(Register::domain(kingdom))?;

let bob_keypair = KeyPair::generate();
let bob = Account::new(bob_id.clone(), [bob_keypair.public_key().clone()]);
let bob = Account::new(bob_id.clone(), bob_keypair.public_key().clone());
test_client.submit_blocking(Register::account(bob))?;

let rabbit = Account::new(rabbit_id.clone(), []);
let rabbit = Account::new_with_random_signature(rabbit_id.clone());
test_client.submit_blocking(Register::account(rabbit))?;

// Grant permission to register asset definitions to "bob@kingdom"
Expand Down Expand Up @@ -228,7 +228,7 @@ fn domain_owner_asset_permissions() -> Result<()> {
test_client.submit_blocking(Register::domain(kingdom))?;

let bob_keypair = KeyPair::generate();
let bob = Account::new(bob_id.clone(), [bob_keypair.public_key().clone()]);
let bob = Account::new(bob_id.clone(), bob_keypair.public_key().clone());
test_client.submit_blocking(Register::account(bob))?;

// Grant permission to register asset definitions to "bob@kingdom"
Expand Down Expand Up @@ -293,7 +293,7 @@ fn domain_owner_trigger_permissions() -> Result<()> {
test_client.submit_blocking(Register::domain(kingdom))?;

let bob_keypair = KeyPair::generate();
let bob = Account::new(bob_id.clone(), [bob_keypair.public_key().clone()]);
let bob = Account::new(bob_id.clone(), bob_keypair.public_key().clone());
test_client.submit_blocking(Register::account(bob))?;

let asset_definition_id = "rose#wonderland".parse()?;
Expand Down Expand Up @@ -354,7 +354,7 @@ fn domain_owner_transfer() -> Result<()> {
test_client.submit_blocking(Register::domain(kingdom))?;

let bob_keypair = KeyPair::generate();
let bob = Account::new(bob_id.clone(), [bob_keypair.public_key().clone()]);
let bob = Account::new(bob_id.clone(), bob_keypair.public_key().clone());
test_client.submit_blocking(Register::account(bob))?;

let domain = test_client.request(FindDomainById::new(kingdom_id.clone()))?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn long_multiple_blocks_created() -> Result<()> {
let create_domain: InstructionBox = Register::domain(Domain::new("domain".parse()?)).into();
let account_id: AccountId = "account@domain".parse()?;
let (public_key, _) = KeyPair::generate().into();
let create_account = Register::account(Account::new(account_id.clone(), [public_key])).into();
let create_account = Register::account(Account::new(account_id.clone(), public_key)).into();
let asset_definition_id: AssetDefinitionId = "xor#domain".parse()?;
let create_asset =
Register::asset_definition(AssetDefinition::quantity(asset_definition_id.clone())).into();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ fn init() -> Result<(
let create_domain = Register::domain(Domain::new("domain".parse()?));
let account_id: AccountId = "account@domain".parse()?;
let (public_key, _) = KeyPair::generate().into();
let create_account = Register::account(Account::new(account_id.clone(), [public_key]));
let create_account = Register::account(Account::new(account_id.clone(), public_key));
let asset_definition_id: AssetDefinitionId = "xor#domain".parse()?;
let create_asset =
Register::asset_definition(AssetDefinition::quantity(asset_definition_id.clone()));
Expand Down
4 changes: 2 additions & 2 deletions client/tests/integration/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ fn permissions_differ_not_only_by_names() {
// Registering mouse
let outfit_domain: DomainId = "outfit".parse().unwrap();
let create_outfit_domain = Register::domain(Domain::new(outfit_domain.clone()));
let new_mouse_account = Account::new(mouse_id.clone(), [mouse_keypair.public_key().clone()]);
let new_mouse_account = Account::new(mouse_id.clone(), mouse_keypair.public_key().clone());
client
.submit_all_blocking([
InstructionBox::from(create_outfit_domain),
Expand Down Expand Up @@ -306,7 +306,7 @@ fn stored_vs_granted_token_payload() -> Result<()> {
Register::asset_definition(AssetDefinition::store(asset_definition_id.clone()));
let mouse_id: AccountId = "mouse@wonderland".parse().expect("Valid");
let mouse_keypair = KeyPair::generate();
let new_mouse_account = Account::new(mouse_id.clone(), [mouse_keypair.public_key().clone()]);
let new_mouse_account = Account::new(mouse_id.clone(), mouse_keypair.public_key().clone());
let instructions: [InstructionBox; 2] = [
Register::account(new_mouse_account).into(),
create_asset.into(),
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/queries/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn find_accounts_with_asset() -> Result<()> {
.iter()
.skip(1) // Alice has already been registered in genesis
.cloned()
.map(|account_id| Register::account(Account::new(account_id, [])))
.map(|account_id| Register::account(Account::new_with_random_signature(account_id)))
.collect::<Vec<_>>();
test_client.submit_all_blocking(register_accounts)?;

Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/queries/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn find_asset_total_quantity() -> Result<()> {
.skip(1) // Alice has already been registered in genesis
.cloned()
.zip(keys.iter().map(KeyPair::public_key).cloned())
.map(|(account_id, public_key)| Register::account(Account::new(account_id, [public_key])))
.map(|(account_id, public_key)| Register::account(Account::new(account_id, public_key)))
.collect::<Vec<_>>();
test_client.submit_all_blocking(register_accounts)?;

Expand Down
4 changes: 2 additions & 2 deletions client/tests/integration/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn register_and_grant_role_for_metadata_access() -> Result<()> {
let mouse_key_pair = KeyPair::generate();
let register_mouse = Register::account(Account::new(
mouse_id.clone(),
[mouse_key_pair.public_key().clone()],
mouse_key_pair.public_key().clone(),
));
test_client.submit_blocking(register_mouse)?;

Expand Down Expand Up @@ -110,7 +110,7 @@ fn unregistered_role_removed_from_account() -> Result<()> {
let mouse_id: AccountId = "mouse@wonderland".parse().expect("Valid");

// Registering Mouse
let register_mouse = Register::account(Account::new(mouse_id.clone(), []));
let register_mouse = Register::account(Account::new_with_random_signature(mouse_id.clone()));
test_client.submit_blocking(register_mouse)?;

// Register root role
Expand Down
8 changes: 5 additions & 3 deletions client/tests/integration/sorting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ fn correct_sorting_of_entities() {
MetadataLimits::new(10, 28),
)
.expect("Valid");
let account = Account::new(account_id.clone(), []).with_metadata(account_metadata.clone());
let account = Account::new_with_random_signature(account_id.clone())
.with_metadata(account_metadata.clone());

accounts.push(account_id);
metadata_of_accounts.push(account_metadata);
Expand Down Expand Up @@ -342,7 +343,7 @@ fn sort_only_elements_which_have_sorting_key() -> Result<()> {
for i in 0..n {
let account_id = AccountId::from_str(&format!("charlie{i}@wonderland")).expect("Valid");
let account = if skip_set.contains(&i) {
let account = Account::new(account_id.clone(), []);
let account = Account::new_with_random_signature(account_id.clone());
accounts_b.push(account_id);
account
} else {
Expand All @@ -354,7 +355,8 @@ fn sort_only_elements_which_have_sorting_key() -> Result<()> {
MetadataLimits::new(10, 28),
)
.expect("Valid");
let account = Account::new(account_id.clone(), []).with_metadata(account_metadata);
let account = Account::new_with_random_signature(account_id.clone())
.with_metadata(account_metadata);
accounts_a.push(account_id);
account
};
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/transfer_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,5 +184,5 @@ fn generate_two_ids() -> (AccountId, AccountId) {

fn create_mouse(mouse_id: AccountId) -> Register<Account> {
let (mouse_public_key, _) = KeyPair::generate().into();
Register::account(Account::new(mouse_id, [mouse_public_key]))
Register::account(Account::new(mouse_id, mouse_public_key))
}
12 changes: 5 additions & 7 deletions client/tests/integration/triggers/data_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ fn must_execute_both_triggers() -> Result<()> {
));
test_client.submit_blocking(register_trigger)?;

test_client.submit_blocking(Register::account(Account::new(
test_client.submit_blocking(Register::account(Account::new_with_random_signature(
"bunny@wonderland".parse()?,
[],
)))?;
test_client.submit_blocking(Register::domain(Domain::new("neverland".parse()?)))?;

Expand All @@ -68,7 +67,8 @@ fn domain_scoped_trigger_must_be_executed_only_on_events_in_its_domain() -> Resu
Register::domain(Domain::new("neverland".parse()?)).into();

let account_id: AccountId = "sapporo@neverland".parse()?;
let create_sapporo_account = Register::account(Account::new(account_id.clone(), [])).into();
let create_sapporo_account =
Register::account(Account::new_with_random_signature(account_id.clone())).into();

let asset_definition_id: AssetDefinitionId = "sakura#neverland".parse()?;
let create_sakura_asset_definition =
Expand Down Expand Up @@ -107,14 +107,12 @@ fn domain_scoped_trigger_must_be_executed_only_on_events_in_its_domain() -> Resu
));
test_client.submit_blocking(register_trigger)?;

test_client.submit_blocking(Register::account(Account::new(
test_client.submit_blocking(Register::account(Account::new_with_random_signature(
"asahi@wonderland".parse()?,
[],
)))?;

test_client.submit_blocking(Register::account(Account::new(
test_client.submit_blocking(Register::account(Account::new_with_random_signature(
"asahi@neverland".parse()?,
[],
)))?;

let new_value = get_asset_value(&test_client, asset_id)?;
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/triggers/time_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ fn mint_nft_for_every_user_every_1_sec() -> Result<()> {
.iter()
.skip(1) // Alice has already been registered in genesis
.cloned()
.map(|account_id| Register::account(Account::new(account_id, [])))
.map(|account_id| Register::account(Account::new_with_random_signature(account_id)))
.collect::<Vec<_>>();
test_client.submit_all_blocking(register_accounts)?;

Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn executor_upgrade_should_work() -> Result<()> {

let admin_id: AccountId = "admin@admin".parse()?;
let admin_keypair = KeyPair::generate();
let admin_account = Account::new(admin_id.clone(), [admin_keypair.public_key().clone()]);
let admin_account = Account::new(admin_id.clone(), admin_keypair.public_key().clone());
let register_admin_account = Register::account(admin_account);
client.submit_blocking(register_admin_account)?;

Expand Down
2 changes: 1 addition & 1 deletion client_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ mod account {
fn run(self, context: &mut dyn RunContext) -> Result<()> {
let Self { id, key, metadata } = self;
let create_account =
iroha_client::data_model::isi::Register::account(Account::new(id, [key]));
iroha_client::data_model::isi::Register::account(Account::new(id, key));
submit([create_account], metadata.load()?, context)
.wrap_err("Failed to register account")
}
Expand Down
Binary file modified configs/swarm/executor.wasm
Binary file not shown.
6 changes: 3 additions & 3 deletions core/benches/blocks/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub fn populate_wsv(
instructions.push(can_unregister_domain.into());
for j in 0..accounts_per_domain {
let account_id = construct_account_id(j, domain_id.clone());
let account = Account::new(account_id.clone(), []);
let account = Account::new_with_random_signature(account_id.clone());
instructions.push(Register::account(account).into());
let can_unregister_account = Grant::permission(
PermissionToken::new(
Expand Down Expand Up @@ -150,7 +150,7 @@ pub fn restore_every_nth(
for j in 0..accounts_per_domain {
if j % nth == 0 || i % nth == 0 {
let account_id = construct_account_id(j, domain_id.clone());
let account = Account::new(account_id.clone(), []);
let account = Account::new_with_random_signature(account_id.clone());
instructions.push(Register::account(account).into());
}
}
Expand Down Expand Up @@ -181,7 +181,7 @@ pub fn build_wsv(
let mut domain = Domain::new(account_id.domain_id.clone()).build(account_id);
domain.accounts.insert(
account_id.clone(),
Account::new(account_id.clone(), [key_pair.public_key().clone()]).build(account_id),
Account::new(account_id.clone(), key_pair.public_key().clone()).build(account_id),
);
let mut wsv = WorldStateView::new(World::with([domain], UniqueVec::new()), kura, query_handle);
wsv.config.transaction_limits = TransactionLimits::new(u64::MAX, u64::MAX);
Expand Down
Loading

0 comments on commit 183ea07

Please sign in to comment.