Skip to content

Commit

Permalink
Verify public key for ETH-implicit account
Browse files Browse the repository at this point in the history
  • Loading branch information
staffik committed Oct 23, 2023
1 parent 7f138a4 commit 36a58be
Show file tree
Hide file tree
Showing 15 changed files with 143 additions and 103 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion chain/rosetta-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ async fn construction_derive(
let public_key: near_crypto::PublicKey = (&public_key)
.try_into()
.map_err(|_| errors::ErrorKind::InvalidInput("Invalid PublicKey".to_string()))?;
// TODO handle eth-implicit accounts
// TODO should ETH-implicit account be handled there?
let address = if let near_crypto::KeyType::ED25519 = public_key.key_type() {
hex::encode(public_key.key_data())
} else {
Expand Down
32 changes: 21 additions & 11 deletions core/account-id/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,22 @@ pub use errors::{ParseAccountError, ParseErrorKind};
#[derive(Eq, Ord, Hash, Clone, Debug, PartialEq, PartialOrd)]
pub struct AccountId(Box<str>);

#[derive(PartialEq)]
pub enum AccountType {
NamedAccount,
NearImplicitAccount,
EthImplicitAccount,
}

impl AccountType {
pub fn is_implicit(&self) -> bool {
match &self {
Self::NearImplicitAccount | Self::EthImplicitAccount => true,
Self::NamedAccount => false,
}
}
}

impl AccountId {
/// Shortest valid length for a NEAR Account ID.
pub const MIN_LEN: usize = 2;
Expand Down Expand Up @@ -133,13 +143,12 @@ impl AccountId {
/// assert!(!alice_app.is_sub_account_of(&near_tla));
/// ```
pub fn is_sub_account_of(&self, parent: &AccountId) -> bool {
// TODO what if this account type is EthImplicitAccount ?
// TODO what if this is EthImplicitAccount ?
self.strip_suffix(parent.as_str())
.and_then(|s| s.strip_suffix('.'))
.map_or(false, |s| !s.contains('.'))
}

// TODO change docs
/// Returns `true` if the `AccountId` is a 40 characters long hexadecimal, possibly with capital letters.
///
/// See [Implicit-Accounts](https://docs.near.org/docs/concepts/account#implicit-accounts).
Expand All @@ -152,14 +161,15 @@ impl AccountId {
/// let alice: AccountId = "alice.near".parse().unwrap();
/// assert!(!alice.is_eth_implicit());
///
/// let rando = "b794f5eA0ba39494ce839613FFfba74279579268"
/// let rando = "0xb794f5eA0ba39494ce839613FFfba74279579268"
/// .parse::<AccountId>()
/// .unwrap();
/// assert!(rando.is_eth_implicit());
/// ```
pub fn is_eth_implicit(&self) -> bool {
self.len() == 40
&& self.as_bytes().iter().all(|b| matches!(b, b'a'..=b'f' | b'A'..=b'F' | b'0'..=b'9'))
fn is_eth_implicit(&self) -> bool {
self.len() == 42
&& self.starts_with("0x")
&& self[2..].as_bytes().iter().all(|b| matches!(b, b'a'..=b'f' | b'A'..=b'F' | b'0'..=b'9'))
}

/// Returns `true` if the `AccountId` is a 64 characters long hexadecimal.
Expand All @@ -179,7 +189,7 @@ impl AccountId {
/// .unwrap();
/// assert!(rando.is_near_implicit());
/// ```
pub fn is_near_implicit(&self) -> bool {
fn is_near_implicit(&self) -> bool {
self.len() == 64 && self.as_bytes().iter().all(|b| matches!(b, b'a'..=b'f' | b'0'..=b'9'))
}

Expand Down Expand Up @@ -465,7 +475,7 @@ mod tests {
"b-o_w_e-n",
"no_lols",
"0123456789012345678901234567890123456789012345678901234567890123",
"b794f5eA0ba39494ce839613FFfba74279579268",
"0xb794f5eA0ba39494ce839613FFfba74279579268",
// Valid, but can't be created
"near.a",
];
Expand Down Expand Up @@ -721,7 +731,7 @@ mod tests {
}
}

// TODO add corresponding test for 40 len hex ETH accounts
// TODO add corresponding test for 42 len hex ETH accounts
#[test]
fn test_is_account_id_64_len_hex() {
let valid_64_len_hex_account_ids = &[
Expand All @@ -735,7 +745,7 @@ mod tests {
assert!(
matches!(
valid_account_id.parse::<AccountId>(),
Ok(account_id) if account_id.is_near_implicit()
Ok(account_id) if account_id.get_account_type() == AccountType::NearImplicitAccount
),
"Account ID {} should be valid 64-len hex",
valid_account_id
Expand All @@ -754,7 +764,7 @@ mod tests {
assert!(
!matches!(
invalid_account_id.parse::<AccountId>(),
Ok(account_id) if account_id.is_near_implicit()
Ok(account_id) if account_id.get_account_type() == AccountType::NearImplicitAccount
),
"Account ID {} is not an implicit account",
invalid_account_id
Expand Down
4 changes: 3 additions & 1 deletion core/crypto/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use curve25519_dalek::traits::VartimeMultiscalarMul;
pub use curve25519_dalek::ristretto::RistrettoPoint as Point;
pub use curve25519_dalek::scalar::Scalar;

use near_account_id::AccountType;

pub fn vmul2(s1: Scalar, p1: &Point, s2: Scalar, p2: &Point) -> Point {
Point::vartime_multiscalar_mul(&[s1, s2], [p1, p2].iter().copied())
}
Expand Down Expand Up @@ -104,7 +106,7 @@ impl PublicKey {
pub fn from_near_implicit_account(
account_id: &near_account_id::AccountId,
) -> Result<Self, ImplicitPublicKeyError> {
if !account_id.is_near_implicit() {
if account_id.get_account_type() != AccountType::NearImplicitAccount {
return Err(ImplicitPublicKeyError::AccountIsNotNearImplicit {
account_id: account_id.clone(),
});
Expand Down
11 changes: 6 additions & 5 deletions core/primitives-core/src/runtime/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::config::ActionCosts;
use crate::num_rational::Rational32;
use crate::types::{Balance, Gas};
use enum_map::EnumMap;
use near_account_id::AccountType;

/// Costs associated with an object that can only be sent over the network (and executed
/// by the receiver).
Expand Down Expand Up @@ -203,17 +204,17 @@ impl StorageUsageConfig {

/// Helper functions for computing Transfer fees.
/// In case of implicit account creation they always include extra fees for the CreateAccount and
/// AddFullAccessKey (except ETH-implicit account) actions that are implicit.
/// AddFullAccessKey (for NEAR-implicit account only) actions that are implicit.
/// We can assume that no overflow will happen here.
pub fn transfer_exec_fee(
cfg: &RuntimeFeesConfig,
is_receiver_implicit: bool,
is_receiver_eth_implictit: bool,
receiver_account_type: AccountType,
) -> Gas {
let mut result = cfg.fee(ActionCosts::transfer).exec_fee();
if is_receiver_implicit {
result += cfg.fee(ActionCosts::create_account).exec_fee();
if !is_receiver_eth_implictit {
if receiver_account_type != AccountType::EthImplicitAccount {
result += cfg.fee(ActionCosts::add_full_access_key).exec_fee();
}
}
Expand All @@ -224,12 +225,12 @@ pub fn transfer_send_fee(
cfg: &RuntimeFeesConfig,
sender_is_receiver: bool,
is_receiver_implicit: bool,
is_receiver_eth_implictit: bool,
receiver_account_type: AccountType,
) -> Gas {
let mut result = cfg.fee(ActionCosts::transfer).send_fee(sender_is_receiver);
if is_receiver_implicit {
result += cfg.fee(ActionCosts::create_account).send_fee(sender_is_receiver);
if !is_receiver_eth_implictit {
if receiver_account_type != AccountType::EthImplicitAccount {
result += cfg.fee(ActionCosts::add_full_access_key).send_fee(sender_is_receiver);
}
}
Expand Down
11 changes: 9 additions & 2 deletions core/primitives/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ pub enum InvalidAccessKeyError {
},
/// Having a deposit with a function call action is not allowed with a function call access key.
DepositWithFunctionCall,
/// Transaction is from ETH-implicit `account_id` and uses invalid `public_key` for that address.
InvalidPkForEthAddress { account_id: AccountId, public_key: PublicKey },
}

/// Describes the error for validating a list of actions.
Expand Down Expand Up @@ -485,7 +487,7 @@ pub enum ActionErrorKind {
/// receipt validation.
NewReceiptValidationError(ReceiptValidationError),
/// Error occurs when a `CreateAccount` action is called on hex-characters
/// account of length 64 or 40. See implicit account creation NEP:
/// account of length 64 or 42. See implicit account creation NEP:
/// <https://github.com/nearprotocol/NEPs/pull/71>.
///
/// TODO(#8598): This error is named very poorly. A better name would be
Expand Down Expand Up @@ -609,7 +611,12 @@ impl Display for InvalidAccessKeyError {
),
InvalidAccessKeyError::DepositWithFunctionCall => {
write!(f, "Having a deposit with a function call action is not allowed with a function call access key.")
}
},
InvalidAccessKeyError::InvalidPkForEthAddress { account_id, public_key } => write!(
f,
"Address {:?} is ETH-implicit and does not correspond to the public_key {}",
account_id, public_key
),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn test_create_top_level_accounts() {
.build();

// These accounts cannot be created because they are top level accounts that are not implicit.
// Note that implicit accounts have to be 64 or 40 characters long.
// Note that implicit accounts have to be 64 or 42 characters long.
let top_level_accounts = [
"0x06012c8cf97bead5deae237070f9587f8e7a266da",
"0a5e97870f263700f46aa00d967821199b9bc5a120",
Expand Down
10 changes: 3 additions & 7 deletions runtime/near-vm-runner/src/logic/logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use super::{StorageGetMode, ValuePtr};
use crate::config::Config;
use crate::ProfileDataV3;
use near_crypto::Secp256K1Signature;
use near_primitives_core::account::id::AccountType;
use near_primitives_core::config::ExtCosts::*;
use near_primitives_core::config::ViewConfig;
use near_primitives_core::config::{ActionCosts, ExtCosts};
Expand Down Expand Up @@ -1774,14 +1773,11 @@ impl<'a> VMLogic<'a> {

let (receipt_idx, sir) = self.promise_idx_to_receipt_idx_with_sir(promise_idx)?;
let receiver_id = self.ext.get_receipt_receiver(receipt_idx);
let is_receiver_implicit = match receiver_id.get_account_type() {
AccountType::NearImplicitAccount | AccountType::EthImplicitAccount => self.config.implicit_account_creation,
AccountType::NamedAccount => false,
};
let is_receiver_implicit = self.config.implicit_account_creation && receiver_id.get_account_type().is_implicit();
let send_fee =
transfer_send_fee(self.fees_config, sir, is_receiver_implicit, receiver_id.is_eth_implicit());
transfer_send_fee(self.fees_config, sir, is_receiver_implicit, receiver_id.get_account_type());
let exec_fee =
transfer_exec_fee(self.fees_config, is_receiver_implicit, receiver_id.is_eth_implicit());
transfer_exec_fee(self.fees_config, is_receiver_implicit, receiver_id.get_account_type());
let burn_gas = send_fee;
let use_gas = burn_gas.checked_add(exec_fee).ok_or(HostError::IntegerOverflow)?;
self.gas_counter.pay_action_accumulated(burn_gas, use_gas, ActionCosts::transfer)?;
Expand Down
1 change: 1 addition & 0 deletions runtime/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ rayon.workspace = true
serde.workspace = true
serde_json.workspace = true
sha2.workspace = true
sha3.workspace = true
thiserror.workspace = true
tracing.workspace = true

Expand Down
74 changes: 40 additions & 34 deletions runtime/runtime/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,32 +441,46 @@ pub(crate) fn action_implicit_account_creation_transfer(
) {
*actor_id = account_id.clone();

let mut access_key = AccessKey::full_access();
// Set default nonce for newly created access key to avoid transaction hash collision.
// See <https://github.com/near/nearcore/issues/3779>.
if checked_feature!("stable", AccessKeyNonceForImplicitAccounts, current_protocol_version) {
access_key.nonce = (block_height - 1)
* near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER;
}

// TODO add support for creating ETH-implicit accounts

// Invariant: The account_id is hex like (implicit account id).
// It holds because in the only calling site, we've checked the permissions before.
// unwrap: Can only fail if `account_id` is not NEAR-implicit.
let public_key = PublicKey::from_near_implicit_account(account_id).unwrap();
match account_id.get_account_type() {
AccountType::NearImplicitAccount => {
let mut access_key = AccessKey::full_access();
// Set default nonce for newly created access key to avoid transaction hash collision.
// See <https://github.com/near/nearcore/issues/3779>.
if checked_feature!("stable", AccessKeyNonceForImplicitAccounts, current_protocol_version) {
access_key.nonce = (block_height - 1)
* near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER;
}

*account = Some(Account::new(
transfer.deposit,
0,
CryptoHash::default(),
fee_config.storage_usage_config.num_bytes_account
+ public_key.len() as u64
+ borsh::object_length(&access_key).unwrap() as u64
+ fee_config.storage_usage_config.num_extra_bytes_record,
));
// Invariant: The account_id is hex like (implicit account id).
// It holds because in the only calling site, we've checked the permissions before.
// unwrap: Can only fail if `account_id` is not NEAR-implicit.
let public_key = PublicKey::from_near_implicit_account(account_id).unwrap();

*account = Some(Account::new(
transfer.deposit,
0,
CryptoHash::default(),
fee_config.storage_usage_config.num_bytes_account
+ public_key.len() as u64
+ borsh::object_length(&access_key).unwrap() as u64
+ fee_config.storage_usage_config.num_extra_bytes_record,
));

set_access_key(state_update, account_id.clone(), public_key, &access_key);
},
AccountType::EthImplicitAccount => {
*account = Some(Account::new(
transfer.deposit,
0,
CryptoHash::default(),
fee_config.storage_usage_config.num_bytes_account
+ fee_config.storage_usage_config.num_extra_bytes_record,
));
},
AccountType::NamedAccount => panic!("Should only be called for an implicit account"),
}

set_access_key(state_update, account_id.clone(), public_key, &access_key);

}

pub(crate) fn action_deploy_contract(
Expand Down Expand Up @@ -885,12 +899,8 @@ pub(crate) fn check_account_existence(
}
.into());
} else {
let is_implicit = match account_id.get_account_type() {
AccountType::NearImplicitAccount | AccountType::EthImplicitAccount => true,
AccountType::NamedAccount => false,
};
// TODO: this should be `config.implicit_account_creation`.
if config.wasm_config.implicit_account_creation && is_implicit {
if config.wasm_config.implicit_account_creation && account_id.get_account_type().is_implicit() {
// If the account doesn't exist and it's implicit, then you
// should only be able to create it using single transfer action.
// Because you should not be able to add another access key to the account in
Expand All @@ -909,13 +919,9 @@ pub(crate) fn check_account_existence(
}
Action::Transfer(_) => {
if account.is_none() {
let is_implicit = match account_id.get_account_type() {
AccountType::NearImplicitAccount | AccountType::EthImplicitAccount => true,
AccountType::NamedAccount => false,
};
return if config.wasm_config.implicit_account_creation
&& is_the_only_action
&& is_implicit
&& account_id.get_account_type().is_implicit()
&& !is_refund
{
// OK. It's implicit account creation.
Expand Down
14 changes: 4 additions & 10 deletions runtime/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,12 @@ pub fn total_send_fees(
}
Transfer(_) => {
// Account for implicit account creation
let is_receiver_implicit = match receiver_id.get_account_type() {
AccountType::NearImplicitAccount | AccountType::EthImplicitAccount => config.wasm_config.implicit_account_creation,
AccountType::NamedAccount => false,
};
let is_receiver_implicit = config.wasm_config.implicit_account_creation && receiver_id.get_account_type().is_implicit();
transfer_send_fee(
fees,
sender_is_receiver,
is_receiver_implicit,
receiver_id.is_eth_implicit(),
receiver_id.get_account_type(),
)
}
Stake(_) => fees.fee(ActionCosts::stake).send_fee(sender_is_receiver),
Expand Down Expand Up @@ -196,11 +193,8 @@ pub fn exec_fee(config: &RuntimeConfig, action: &Action, receiver_id: &AccountId
}
Transfer(_) => {
// Account for implicit account creation
let is_receiver_implicit = match receiver_id.get_account_type() {
AccountType::NearImplicitAccount | AccountType::EthImplicitAccount => config.wasm_config.implicit_account_creation,
AccountType::NamedAccount => false,
};
transfer_exec_fee(fees, is_receiver_implicit, receiver_id.is_eth_implicit())
let is_receiver_implicit = config.wasm_config.implicit_account_creation && receiver_id.get_account_type().is_implicit();
transfer_exec_fee(fees, is_receiver_implicit, receiver_id.get_account_type())
}
Stake(_) => fees.fee(ActionCosts::stake).exec_fee(),
AddKey(add_key_action) => match &add_key_action.access_key.permission {
Expand Down
Loading

0 comments on commit 36a58be

Please sign in to comment.