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 dd17891
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 66 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.

23 changes: 17 additions & 6 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 @@ -157,9 +167,10 @@ impl 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 +190,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 @@ -735,7 +746,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 +765,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
9 changes: 5 additions & 4 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 @@ -208,12 +209,12 @@ impl StorageUsageConfig {
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
9 changes: 8 additions & 1 deletion 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 @@ -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
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
62 changes: 38 additions & 24 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
4 changes: 2 additions & 2 deletions runtime/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub fn total_send_fees(
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 @@ -200,7 +200,7 @@ pub fn exec_fee(config: &RuntimeConfig, action: &Action, receiver_id: &AccountId
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())
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
53 changes: 41 additions & 12 deletions runtime/runtime/src/verifier.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::config::{total_prepaid_gas, tx_cost, TransactionCost};
use crate::near_primitives::account::Account;
use crate::VerificationResult;
use near_crypto::{PublicKey, KeyType};
use near_crypto::key_conversion::is_valid_staking_key;
use near_primitives::account::AccessKeyPermission;
use near_primitives::account::{AccessKey, AccessKeyPermission};
use near_primitives::action::delegate::SignedDelegateAction;
use near_primitives::checked_feature;
use near_primitives::errors::{
Expand All @@ -19,6 +20,7 @@ use near_primitives::types::{AccountId, Balance};
use near_primitives::types::{BlockHeight, StorageUsage};
use near_primitives::version::ProtocolFeature;
use near_primitives::version::ProtocolVersion;
use near_primitives_core::account::id::AccountType;
use near_store::{
get_access_key, get_account, set_access_key, set_account, StorageError, TrieUpdate,
};
Expand Down Expand Up @@ -123,6 +125,18 @@ pub fn validate_transaction(
.map_err(|_| InvalidTxError::CostOverflow.into())
}

fn is_pk_valid_for_eth_address(public_key: &PublicKey, account_id: &AccountId) -> bool {
match public_key.key_type() {
KeyType::SECP256K1 => {
use sha3::Digest;
let pk_hash = sha3::Keccak256::digest(&public_key.key_data());
format!("0x{}", hex::encode(&pk_hash[12..32])) == account_id.as_str()
},
// Should panic instead?
KeyType::ED25519 => false,
}
}

/// Verifies the signed transaction on top of given state, charges transaction fees
/// and balances, and updates the state for the used account and access keys.
pub fn verify_and_charge_transaction(
Expand All @@ -144,23 +158,38 @@ pub fn verify_and_charge_transaction(
)?;
let transaction = &signed_transaction.transaction;
let signer_id = &transaction.signer_id;
let public_key = &transaction.public_key;

let mut signer = match get_account(state_update, signer_id)? {
Some(signer) => signer,
None => {
return Err(InvalidTxError::SignerDoesNotExist { signer_id: signer_id.clone() }.into());
}
};
let mut access_key = match get_access_key(state_update, signer_id, &transaction.public_key)? {
let mut access_key = match get_access_key(state_update, signer_id, public_key)? {
Some(access_key) => access_key,
None => {
return Err(InvalidTxError::InvalidAccessKeyError(
InvalidAccessKeyError::AccessKeyNotFound {
account_id: signer_id.clone(),
public_key: transaction.public_key.clone(),
},
)
.into());
None => match signer_id.get_account_type() {
AccountType::EthImplicitAccount => {
if is_pk_valid_for_eth_address(public_key, signer_id) {
// TODO what about increasing storage usage for that account
AccessKey::full_access()
} else {
return Err(InvalidTxError::InvalidAccessKeyError(
InvalidAccessKeyError::InvalidPkForEthAddress {
account_id: signer_id.clone(),
public_key: public_key.clone(),
})
.into());
}
},
_ => {
return Err(InvalidTxError::InvalidAccessKeyError(
InvalidAccessKeyError::AccessKeyNotFound {
account_id: signer_id.clone(),
public_key: public_key.clone(),
})
.into());
},
}
};

Expand Down Expand Up @@ -202,7 +231,7 @@ pub fn verify_and_charge_transaction(
*allowance = allowance.checked_sub(total_cost).ok_or_else(|| {
InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::NotEnoughAllowance {
account_id: signer_id.clone(),
public_key: transaction.public_key.clone(),
public_key: public_key.clone(),
allowance: *allowance,
cost: total_cost,
})
Expand Down Expand Up @@ -268,7 +297,7 @@ pub fn verify_and_charge_transaction(
}
};

set_access_key(state_update, signer_id.clone(), transaction.public_key.clone(), &access_key);
set_access_key(state_update, signer_id.clone(), public_key.clone(), &access_key);
set_account(state_update, signer_id.clone(), &signer);

Ok(VerificationResult { gas_burnt, gas_remaining, receipt_gas_price, burnt_amount })
Expand Down
Loading

0 comments on commit dd17891

Please sign in to comment.