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

feat: storage bytecode stored in 31-bytes chunks from address 0 #838

Merged
merged 2 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 7 additions & 7 deletions crates/contracts/src/account_contract.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub mod AccountContract {
NONCE_READ_ERROR, NONCE_WRITE_ERROR, KAKAROT_VALIDATION_FAILED
};
use contracts::kakarot_core::interface::{IKakarotCoreDispatcher, IKakarotCoreDispatcherTrait};
use contracts::storage::StorageBytecode;
use core::integer;
use core::num::traits::Bounded;
use core::num::traits::zero::Zero;
Expand Down Expand Up @@ -82,8 +83,9 @@ pub mod AccountContract {


#[storage]
struct Storage {
Account_bytecode: ByteArray,
pub(crate) struct Storage {
Account_bytecode: StorageBytecode,
pub(crate) Account_bytecode_len: u32,
Account_storage: Map<u256, u256>,
Account_is_initialized: bool,
Account_nonce: u64,
Expand Down Expand Up @@ -157,7 +159,7 @@ pub mod AccountContract {
assert(calls.len() == 1, 'EOA: multicall not supported');
// todo: activate check once using snfoundry
// assert(tx_info.version.try_into().unwrap() >= 1_u128, 'EOA: deprecated tx version');
assert(self.Account_bytecode.read().len().is_zero(), 'EOAs: Cannot have code');
assert(self.Account_bytecode_len.read().is_zero(), 'EOAs: Cannot have code');
assert(tx_info.signature.len() == 5, 'EOA: invalid signature length');

let call = calls.at(0);
Expand Down Expand Up @@ -242,13 +244,11 @@ pub mod AccountContract {

fn write_bytecode(ref self: ContractState, bytecode: Span<u8>) {
self.ownable.assert_only_owner();
let packed_bytecode: ByteArray = ByteArrayExTrait::from_bytes(bytecode);
self.Account_bytecode.write(packed_bytecode);
self.Account_bytecode.write(StorageBytecode { bytecode });
}

fn bytecode(self: @ContractState) -> Span<u8> {
let packed_bytecode = self.Account_bytecode.read();
packed_bytecode.into_bytes()
self.Account_bytecode.read().bytecode
}

fn write_storage(ref self: ContractState, key: u256, value: u256) {
Expand Down
14 changes: 7 additions & 7 deletions crates/contracts/src/cairo1_helpers.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ pub trait IHelpers<T> {
mod embeddable_impls {
use core::keccak::{cairo_keccak, keccak_u256s_be_inputs};
use core::num::traits::Zero;
use core::starknet::EthAddress;
use core::starknet::eth_signature::{
Signature, verify_eth_signature, public_key_point_to_eth_address
};
use core::starknet::secp256_trait::{recover_public_key, Secp256PointTrait, is_valid_signature};
use core::starknet::secp256k1::Secp256k1Point;
use core::starknet::secp256r1::{secp256r1_new_syscall, Secp256r1Point};
use core::traits::Into;
use core::{starknet, starknet::SyscallResultTrait};
use evm::errors::EVMError;
Expand All @@ -117,13 +124,6 @@ mod embeddable_impls {
use evm::precompiles::identity::Identity;
use evm::precompiles::modexp::ModExp;
use evm::precompiles::sha256::Sha256;
use core::starknet::EthAddress;
use core::starknet::eth_signature::{
Signature, verify_eth_signature, public_key_point_to_eth_address
};
use core::starknet::secp256_trait::{recover_public_key, Secp256PointTrait, is_valid_signature};
use core::starknet::secp256k1::Secp256k1Point;
use core::starknet::secp256r1::{secp256r1_new_syscall, Secp256r1Point};
use utils::helpers::U256Trait;


Expand Down
10 changes: 5 additions & 5 deletions crates/contracts/src/kakarot_core/kakarot.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ pub mod KakarotCore {
Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess,
StoragePointerWriteAccess
};
use core::starknet::syscalls::deploy_syscall;
use core::starknet::{
EthAddress, ContractAddress, ClassHash, get_tx_info, get_contract_address,
get_caller_address
};
use evm::backend::starknet_backend;
use evm::errors::{EVMError, ensure, EVMErrorTrait,};
use evm::gas;
Expand All @@ -26,11 +31,6 @@ pub mod KakarotCore {
ExecutionSummaryTrait, Address, AddressTrait
};
use evm::state::{State, StateTrait};
use core::starknet::syscalls::deploy_syscall;
use core::starknet::{
EthAddress, ContractAddress, ClassHash, get_tx_info, get_contract_address,
get_caller_address
};
use super::{INVOKE_ETH_CALL_FORBIDDEN};
use utils::address::compute_contract_address;
use utils::constants;
Expand Down
2 changes: 2 additions & 0 deletions crates/contracts/src/lib.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ mod errors;
// Kakarot smart contract
mod kakarot_core;

mod storage;

#[cfg(target: 'test')]
mod test_data;

Expand Down
205 changes: 205 additions & 0 deletions crates/contracts/src/storage.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
use contracts::account_contract::AccountContract::unsafe_new_contract_state as account_contract_state;
use core::ops::DerefMut;
use core::ops::SnapshotDeref;
use core::starknet::{
SyscallResult, storage_read_syscall, Store, StorageBaseAddress, StorageAddress,
storage_write_syscall
};
use starknet::storage::StorageTrait;
use starknet::storage::StorageTraitMut;
use super::account_contract::IAccount;
use utils::utils::{pack_bytes, load_packed_bytes};

/// A wrapper type for the bytecode storage. Packing / unpacking is done transparently inside the
/// `read` and `write` methods of `Store`.
#[derive(Copy, Drop)]
struct StorageBytecode {
bytecode: Span<u8>
}

const BYTES_PER_FELT: NonZero<u32> = 31;

/// An implamentation of the `Store` trait for our specific `StorageBytecode` type.
/// The packing-unpacking is done inside the `read` and `write` methods, thus transparent to the
/// user.
/// The bytecode is stored sequentially, starting from storage address 0, for compatibility purposes
/// with KakarotZero.
/// The bytecode length is stored in the `Account_bytecode_len` storage variable, which is accessed
/// by the `read` and `write` methods.
impl StoreBytecode of Store<StorageBytecode> {
/// Side effect: reads the bytecode len from the Account_bytecode_len storage variable
fn read(address_domain: u32, base: StorageBaseAddress) -> SyscallResult<StorageBytecode> {
// Read the bytecode len from the storage of the current contract
let state = account_contract_state();
let bytecode_len: u32 = state.snapshot_deref().storage().Account_bytecode_len.read();
let (chunks_count, _remainder) = DivRem::div_rem(bytecode_len, BYTES_PER_FELT);

// Read the bytecode from the storage of the current contract, starting from address 0.
//TODO(opti): unpack chunks directly instead of reading them one by one and unpacking them
// afterwards.
let base: felt252 = 0;
let mut packed_bytecode = array![];
let mut i = 0;
while i != (chunks_count + 1) {
let storage_address: StorageAddress = (base + i.into()).try_into().unwrap();
let chunk = storage_read_syscall(address_domain, storage_address).unwrap();
packed_bytecode.append(chunk);
i += 1;
};
let bytecode = load_packed_bytes(packed_bytecode.span(), bytecode_len);
SyscallResult::Ok(StorageBytecode { bytecode: bytecode.span() })
}

/// Side effect: Writes the bytecode len to the Account_bytecode_len storage variable
fn write(
address_domain: u32, base: StorageBaseAddress, value: StorageBytecode
) -> SyscallResult<()> {
let base: felt252 = 0;
let mut state = account_contract_state();
let bytecode_len: u32 = value.bytecode.len();
state.deref_mut().storage_mut().Account_bytecode_len.write(bytecode_len);

let mut packed_bytecode = pack_bytes(value.bytecode);
let mut i = 0;
for chunk in packed_bytecode {
let storage_address: StorageAddress = (base + i.into()).try_into().unwrap();
storage_write_syscall(address_domain, storage_address, chunk).unwrap();
i += 1;
};
SyscallResult::Ok(())
}

fn read_at_offset(
address_domain: u32, base: StorageBaseAddress, offset: u8
) -> SyscallResult<StorageBytecode> {
panic!("'read_at_offset' is not implemented for StoreBytecode")
}

fn write_at_offset(
address_domain: u32, base: StorageBaseAddress, offset: u8, value: StorageBytecode
) -> SyscallResult<()> {
panic!("'write_at_offset' is not implemented for StoreBytecode")
}

fn size() -> u8 {
panic!("'size' is not implemented for StoreBytecode")
}
}

#[cfg(test)]
mod tests {
use contracts::account_contract::AccountContract::{
unsafe_new_contract_state as account_contract_state, ContractState as AccountContractState
};
use starknet::contract_address::ContractAddress;
use starknet::storage_access::Store;
use starknet::storage_access::{
StorageBaseAddress, StorageAddress, storage_base_address_from_felt252
};
use starknet::syscalls::storage_read_syscall;
use starknet::testing::set_contract_address;
use super::DerefMut;
use super::SnapshotDeref;
use super::StorageBytecode;
use super::StorageTrait;
use super::StorageTraitMut;
use utils::utils::{pack_bytes, load_packed_bytes};

#[test]
fn test_store_bytecode_empty() {
let mut state = account_contract_state();
let bytecode = [].span();
// Write the bytecode to the storage
state.deref_mut().storage_mut().Account_bytecode.write(StorageBytecode { bytecode });
// Verify that the bytecode was written correctly and the len as well
let bytecode_len = state.snapshot_deref().storage().Account_bytecode_len.read();
let stored_bytecode = state.snapshot_deref().storage().Account_bytecode.read();
assert_eq!(bytecode_len, bytecode.len());
assert_eq!(stored_bytecode.bytecode, bytecode);
}

#[test]
fn test_store_bytecode_single_chunk() {
let mut state = account_contract_state();
let bytecode = [0, 1, 2, 3, 4, 5].span();
// Write the bytecode to the storage
state.deref_mut().storage_mut().Account_bytecode.write(StorageBytecode { bytecode });
// Verify that the bytecode was written correctly and the len as well
let bytecode_len = state.snapshot_deref().storage().Account_bytecode_len.read();
let stored_bytecode = state.snapshot_deref().storage().Account_bytecode.read();
assert_eq!(bytecode_len, bytecode.len());
assert_eq!(stored_bytecode.bytecode, bytecode);
}

#[test]
fn test_store_bytecode_multiple_chunks() {
let mut state = account_contract_state();
let mut bytecode_array = array![];
let mut i = 0;
while i != 100 {
bytecode_array.append(i);
i += 1;
};
let bytecode = bytecode_array.span();
// Write the bytecode to the storage
state.deref_mut().storage_mut().Account_bytecode.write(StorageBytecode { bytecode });
// Verify that the bytecode was written correctly and the len as well
let bytecode_len = state.snapshot_deref().storage().Account_bytecode_len.read();
let stored_bytecode = state.snapshot_deref().storage().Account_bytecode.read();
assert_eq!(bytecode_len, bytecode.len());
assert_eq!(stored_bytecode.bytecode, bytecode);
}

#[test]
fn test_store_bytecode_partial_chunk() {
let mut state = account_contract_state();
let bytecode = [
1
; 33].span(); // 33 bytes will require 2 chunks, with the second chunk partially filled
// Write the bytecode to the storage
state.deref_mut().storage_mut().Account_bytecode.write(StorageBytecode { bytecode });
// Verify that the bytecode was written correctly and the len as well
let bytecode_len = state.snapshot_deref().storage().Account_bytecode_len.read();
let stored_bytecode = state.snapshot_deref().storage().Account_bytecode.read();
assert_eq!(bytecode_len, bytecode.len());
assert_eq!(stored_bytecode.bytecode, bytecode);
}

#[test]
fn test_storage_layout_sequential_from_zero() {
let base_address: StorageAddress = 0.try_into().unwrap();
let bytecode = [0x12; 33].span();
let stored_bytecode = StorageBytecode { bytecode };
let mut state = account_contract_state();
state.deref_mut().storage_mut().Account_bytecode.write(stored_bytecode);

// Verify that the bytecode was packed in chunks sequential from zero
let chunk0 = storage_read_syscall(0, base_address).unwrap();
let chunk1 = storage_read_syscall(0, 1.try_into().unwrap()).unwrap();

assert_eq!(chunk0, (*pack_bytes([0x12; 31].span())[0]));
assert_eq!(chunk1, 0x1212);
}

#[test]
#[should_panic(expected: "'read_at_offset' is not implemented for StoreBytecode")]
fn test_read_at_offset_panics() {
let base_address: StorageBaseAddress = storage_base_address_from_felt252(0);
let _ = Store::<StorageBytecode>::read_at_offset(0, base_address, 0);
}

#[test]
#[should_panic(expected: "'write_at_offset' is not implemented for StoreBytecode")]
fn test_write_at_offset_panics() {
let base_address: StorageBaseAddress = storage_base_address_from_felt252(0);
let bytecode = array![].span();
let stored_bytecode = StorageBytecode { bytecode };
let _ = Store::<StorageBytecode>::write_at_offset(0, base_address, 0, stored_bytecode);
}

#[test]
#[should_panic(expected: "'size' is not implemented for StoreBytecode")]
fn test_size_panics() {
let _ = Store::<StorageBytecode>::size();
}
}
8 changes: 4 additions & 4 deletions crates/contracts/src/test_utils.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ use contracts::kakarot_core::{
use contracts::uninitialized_account::{UninitializedAccount};
use core::fmt::Debug;
use core::result::ResultTrait;
use core::starknet::{
testing, contract_address_const, EthAddress, ContractAddress, deploy_syscall,
get_contract_address
};
use evm::backend::starknet_backend;
use evm::model::{Address};

use evm::test_utils::{ca_address, other_starknet_address, chain_id, sequencer_evm_address};
use openzeppelin::token::erc20::ERC20;
use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait};
use core::starknet::{
testing, contract_address_const, EthAddress, ContractAddress, deploy_syscall,
get_contract_address
};
use utils::constants::BLOCK_GAS_LIMIT;
use utils::eth_transaction::LegacyTransaction;

Expand Down
14 changes: 7 additions & 7 deletions crates/contracts/tests/test_eoa.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ use contracts_tests::test_upgradeable::{
use core::array::SpanTrait;
use core::box::BoxTrait;
use core::starknet::account::{Call};

use evm::model::{Address, AddressTrait};
use evm::test_utils::{
kakarot_address, evm_address, other_evm_address, other_starknet_address, eoa_address, chain_id,
tx_gas_limit, gas_price, VMBuilderTrait
};
use openzeppelin::token::erc20::interface::IERC20CamelDispatcherTrait;
use core::starknet::class_hash::Felt252TryIntoClassHash;
use core::starknet::testing::{
set_caller_address, set_contract_address, set_signature, set_chain_id
Expand All @@ -31,6 +24,13 @@ use core::starknet::{
deploy_syscall, ContractAddress, ClassHash, VALIDATED, get_contract_address,
contract_address_const, EthAddress, eth_signature::{Signature}, get_tx_info
};

use evm::model::{Address, AddressTrait};
use evm::test_utils::{
kakarot_address, evm_address, other_evm_address, other_starknet_address, eoa_address, chain_id,
tx_gas_limit, gas_price, VMBuilderTrait
};
use openzeppelin::token::erc20::interface::IERC20CamelDispatcherTrait;
use utils::eth_transaction::{
TransactionType, EthereumTransaction, EthereumTransactionTrait, LegacyTransaction
};
Expand Down
2 changes: 1 addition & 1 deletion crates/contracts/tests/test_kakarot_core.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ use contracts_tests::test_upgradeable::{
};
use core::num::traits::Zero;
use core::option::OptionTrait;
use core::starknet::{testing, contract_address_const, ContractAddress, EthAddress, ClassHash};


use core::traits::TryInto;
use evm::model::{Address};
use evm::test_utils::{sequencer_evm_address, chain_id};
use evm::test_utils;
use core::starknet::{testing, contract_address_const, ContractAddress, EthAddress, ClassHash};
use utils::eth_transaction::{EthereumTransaction, EthereumTransactionTrait, LegacyTransaction};
use utils::helpers::{EthAddressExTrait, u256_to_bytes_array};

Expand Down
4 changes: 2 additions & 2 deletions crates/contracts/tests/test_ownable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use contracts::components::ownable::{ownable_component};
use contracts::test_utils::constants::{ZERO, OWNER, OTHER};
use contracts::test_utils;
use core::num::traits::Zero;
use core::starknet::ContractAddress;
use core::starknet::testing;


use ownable_component::{InternalImpl, OwnableImpl};
use core::starknet::ContractAddress;
use core::starknet::testing;


#[starknet::contract]
Expand Down
Loading
Loading