Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NDEV-2505 Fix EXTHASHCODE #253

Merged
merged 9 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions evm_loader/Cargo.lock

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

21 changes: 0 additions & 21 deletions evm_loader/lib/src/account_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,27 +623,6 @@ impl<T: Rpc> AccountStorage for EmulatorAccountStorage<'_, T> {
address.find_solana_address(self.program_id())
}

async fn code_hash(&self, address: Address, chain_id: u64) -> [u8; 32] {
use solana_sdk::keccak::hash;

info!("code_hash {address} {chain_id}");

let code = self.code(address).await.to_vec();
if !code.is_empty() {
return hash(&code).to_bytes();
}

// https://eips.ethereum.org/EIPS/eip-1052
// https://eips.ethereum.org/EIPS/eip-161
if (self.balance(address, chain_id).await == 0)
&& (self.nonce(address, chain_id).await == 0)
{
return <[u8; 32]>::default();
}

hash(&[]).to_bytes()
}

async fn code_size(&self, address: Address) -> usize {
info!("code_size {address}");

Expand Down
3 changes: 3 additions & 0 deletions evm_loader/program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ async-trait = { version = "0.1.73", optional = true }
version = "0.2.7"
features = ["is_sync"]

[dev-dependencies]
tokio = { version = "1.0", features = ["full"] }

[lib]
crate-type = ["cdylib", "lib"]

Expand Down
23 changes: 0 additions & 23 deletions evm_loader/program/src/account_storage/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,29 +73,6 @@ impl<'a> AccountStorage for ProgramAccountStorage<'a> {
.contract_with_bump_seed(self.program_id(), address)
}

fn code_hash(&self, address: Address, chain_id: u64) -> [u8; 32] {
use solana_program::keccak;

if let Ok(contract) = self.contract_account(address) {
keccak::hash(&contract.code()).to_bytes()
} else {
// https://eips.ethereum.org/EIPS/eip-1052
// https://eips.ethereum.org/EIPS/eip-161
if let Ok(account) = self.balance_account(address, chain_id) {
if account.nonce() > 0 || account.balance() > 0 {
// account without code
keccak::hash(&[]).to_bytes()
} else {
// non-existent account
<[u8; 32]>::default()
}
} else {
// non-existent account
<[u8; 32]>::default()
}
}
}

fn code_size(&self, address: Address) -> usize {
self.contract_account(address).map_or(0, |a| a.code_len())
}
Expand Down
2 changes: 0 additions & 2 deletions evm_loader/program/src/account_storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ pub trait AccountStorage {
/// Get contract solana address
fn contract_pubkey(&self, address: Address) -> (Pubkey, u8);

/// Get code hash
async fn code_hash(&self, address: Address, chain_id: u64) -> [u8; 32];
/// Get code size
async fn code_size(&self, address: Address) -> usize;
/// Get code data
Expand Down
243 changes: 241 additions & 2 deletions evm_loader/program/src/evm/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ pub trait Database {
chain_id: u64,
value: U256,
) -> Result<()>;

async fn code_hash(&self, address: Address, chain_id: u64) -> Result<[u8; 32]>;
async fn code_size(&self, address: Address) -> Result<usize>;
async fn code(&self, address: Address) -> Result<Buffer>;
fn set_code(&mut self, address: Address, chain_id: u64, code: Vec<u8>) -> Result<()>;
Expand Down Expand Up @@ -51,3 +49,244 @@ pub trait Database {
is_static: bool,
) -> Option<Result<Vec<u8>>>;
}

/// Provides convenience methods that can be implemented in terms of `Database`.
#[maybe_async(?Send)]
pub trait DatabaseExt {
/// Returns whether an account exists, which is also referred to as the account being non-empty,
/// in accordance with https://eips.ethereum.org/EIPS/eip-161.
async fn account_exists(&self, address: Address, chain_id: u64) -> Result<bool>;

/// Returns the code has for an address in accordance with https://eips.ethereum.org/EIPS/eip-1052.
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
async fn code_hash(&self, address: Address, chain_id: u64) -> Result<[u8; 32]>;
}

#[maybe_async(?Send)]
impl<T: Database> DatabaseExt for T {
async fn account_exists(&self, address: Address, chain_id: u64) -> Result<bool> {
Ok(self.nonce(address, chain_id).await? > 0 || self.balance(address, chain_id).await? > 0)
}

async fn code_hash(&self, address: Address, chain_id: u64) -> Result<[u8; 32]> {
// The function `Database::code` returns a zero-length buffer if the account exists with
// zero-length code, but also when the account does not exist. This makes it necessary to
// also check if the account exists when the returned buffer is empty.
//
// We could simplify the implementation by checking if the account exists first, but that
// would lead to more computation in what we think is the common case where the account
// exists and contains code.
let code = self.code(address).await?;
let bytes_to_hash: Option<&[u8]> = if !code.is_empty() {
Some(&*code)
} else if self.account_exists(address, chain_id).await? {
Some(&[])
} else {
None
};

Ok(bytes_to_hash.map_or([0; 32], |bytes| {
solana_program::keccak::hash(bytes).to_bytes()
}))
}
}

#[cfg(test)]
mod tests {
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
use std::collections::HashMap;

use super::*;

struct TestDatabaseEntry {
balance: U256,
nonce: u64,
code: Vec<u8>,
}

#[allow(dead_code)]
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
struct TestDatabase(HashMap<Address, TestDatabaseEntry>);

const NON_EXISTENT_ADDRESS: Address = Address([0xaa; 20]);
const EMPTY_CODE_ADDRESS: Address = Address([0xbb; 20]);
const NON_EMPTY_CODE_ADDRESS: Address = Address([0xcc; 20]);

impl Default for TestDatabase {
fn default() -> Self {
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
Self(
[
(
EMPTY_CODE_ADDRESS,
TestDatabaseEntry {
balance: U256::from_words(0, 1),
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
nonce: 1,
code: Vec::default(),
},
),
(
NON_EMPTY_CODE_ADDRESS,
TestDatabaseEntry {
balance: U256::from_words(0, 1),
nonce: 1,
code: vec![0; 10],
},
),
]
.into_iter()
.collect(),
)
}
}
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved

#[maybe_async(?Send)]
#[allow(unused_variables)]
impl Database for TestDatabase {
fn default_chain_id(&self) -> u64 {
unimplemented!();
}

fn is_valid_chain_id(&self, chain_id: u64) -> bool {
unimplemented!();
}

async fn contract_chain_id(&self, address: Address) -> Result<u64> {
unimplemented!();
}

async fn nonce(&self, address: Address, chain_id: u64) -> Result<u64> {
Ok(self
.0
.get(&address)
.map(|entry| entry.nonce)
.unwrap_or_default())
}

fn increment_nonce(&mut self, address: Address, chain_id: u64) -> Result<()> {
unimplemented!();
}

async fn balance(&self, address: Address, chain_id: u64) -> Result<U256> {
Ok(self
.0
.get(&address)
.map(|entry| entry.balance)
.unwrap_or_default())
}

async fn transfer(
&mut self,
source: Address,
target: Address,
chain_id: u64,
value: U256,
) -> Result<()> {
unimplemented!();
}

async fn code_size(&self, address: Address) -> Result<usize> {
unimplemented!();
}

async fn code(&self, address: Address) -> Result<Buffer> {
Ok(self
.0
.get(&address)
.map(|entry| Buffer::from_slice(&entry.code))
.unwrap_or_default())
}

fn set_code(&mut self, address: Address, chain_id: u64, code: Vec<u8>) -> Result<()> {
unimplemented!();
}

fn selfdestruct(&mut self, address: Address) -> Result<()> {
unimplemented!();
}

async fn storage(&self, address: Address, index: U256) -> Result<[u8; 32]> {
unimplemented!();
}

fn set_storage(&mut self, address: Address, index: U256, value: [u8; 32]) -> Result<()> {
unimplemented!();
}

async fn block_hash(&self, number: U256) -> Result<[u8; 32]> {
unimplemented!();
}

fn block_number(&self) -> Result<U256> {
unimplemented!();
}

fn block_timestamp(&self) -> Result<U256> {
unimplemented!();
}

async fn map_solana_account<F, R>(&self, address: &Pubkey, action: F) -> R
where
F: FnOnce(&AccountInfo) -> R,
{
unimplemented!();
}

fn snapshot(&mut self) {
unimplemented!();
}

fn revert_snapshot(&mut self) {
unimplemented!();
}

fn commit_snapshot(&mut self) {
unimplemented!();
}

async fn precompile_extension(
&mut self,
context: &Context,
address: &Address,
data: &[u8],
is_static: bool,
) -> Option<Result<Vec<u8>>> {
unimplemented!();
}
}

macro_rules! code_hash_test {
($fn_name:ident, $address:expr, $expected:expr) => {
#[maybe_async::test(feature = "is_sync", async(not(feature = "is_sync"), tokio::test))]
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
async fn $fn_name() {
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
let database = TestDatabase::default();

let actual = database
.code_hash($address, crate::config::DEFAULT_CHAIN_ID)
.await
.unwrap();
assert_eq!(actual, $expected);
}
};
}

code_hash_test!(
code_hash_of_non_existing_address,
NON_EXISTENT_ADDRESS,
[0; 32]
);

code_hash_test!(
code_hash_of_empty_contract_address,
EMPTY_CODE_ADDRESS,
[
197, 210, 70, 1, 134, 247, 35, 60, 146, 126, 125, 178, 220, 199, 3, 192, 229, 0, 182,
83, 202, 130, 39, 59, 123, 250, 216, 4, 93, 133, 164, 112
]
);

code_hash_test!(
code_hash_of_non_empty_contract_address,
NON_EMPTY_CODE_ADDRESS,
[
107, 210, 221, 107, 212, 8, 203, 238, 51, 66, 147, 88, 191, 36, 253, 198, 70, 18, 251,
248, 177, 180, 219, 96, 69, 24, 244, 15, 253, 52, 182, 7
]
);
}
5 changes: 4 additions & 1 deletion evm_loader/program/src/evm/opcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use ethnum::{I256, U256};
use maybe_async::maybe_async;
use solana_program::log::sol_log_data;

use super::{database::Database, tracing_event, Context, Machine, Reason};
use super::{
database::{Database, DatabaseExt},
tracing_event, Context, Machine, Reason,
};
use crate::{
error::{Error, Result},
evm::{trace_end_step, Buffer},
Expand Down
14 changes: 0 additions & 14 deletions evm_loader/program/src/executor/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,20 +284,6 @@ impl<'a, B: AccountStorage> Database for ExecutorState<'a, B> {
Ok(self.backend.code_size(from_address).await)
}

async fn code_hash(&self, from_address: Address, chain_id: u64) -> Result<[u8; 32]> {
use solana_program::keccak::hash;

for action in &self.actions {
if let Action::EvmSetCode { address, code, .. } = action {
if &from_address == address {
return Ok(hash(code).to_bytes());
}
}
}

Ok(self.backend.code_hash(from_address, chain_id).await)
mickvangelderen marked this conversation as resolved.
Show resolved Hide resolved
}

async fn code(&self, from_address: Address) -> Result<crate::evm::Buffer> {
for action in &self.actions {
if let Action::EvmSetCode { address, code, .. } = action {
Expand Down
Loading