Skip to content

Commit

Permalink
Pass code_hash to ContractCode when it's available (#3102)
Browse files Browse the repository at this point in the history
Don't re-hash a contract code after a read if the `code_hash` is already available in the account ID. The only place to rehash is when `apply_genesis_records` is called.  

Ref: #3094

## Test plan:

- [x] CI
- [x] Run fee estimator again.

To compare.
**this PR**
```rust
data_receipt_creation_config: DataReceiptCreationConfig {
            base_cost: Fee {
                send_sir: 1035057529250,
                send_not_sir: 1035057529250,
                execution: 1035057529250,
            },
            cost_per_byte: Fee {
                send_sir: 23636040,
                send_not_sir: 23636040,
                execution: 23636040,
            },
        },
```
vs 
```rust
data_receipt_creation_config: DataReceiptCreationConfig {
                base_cost: Fee {
                    send_sir: 4697339419375,
                    send_not_sir: 4697339419375,
                    execution: 4697339419375,
                },
                cost_per_byte: Fee {
                    send_sir: 59357464,
                    send_not_sir: 59357464,
                    execution: 59357464,
                },
            },
```

**this PR**
```rust
function_call_cost: Fee {
                send_sir: 1185240500000,
                send_not_sir: 1185240500000,
                execution: 1185240500000,
            },
            function_call_cost_per_byte: Fee {
                send_sir: 1158368,
                send_not_sir: 1158368,
                execution: 1158368,
            },
```

vs 

```rust
function_call_cost: Fee {
                    send_sir: 2319861500000,
                    send_not_sir: 2319861500000,
                    execution: 2319861500000,
                },
                function_call_cost_per_byte: Fee {
                    send_sir: 2235934,
                    send_not_sir: 2235934,
                    execution: 2235934,
                },
```
  • Loading branch information
Evgeny Kuzyakov authored Aug 9, 2020
1 parent b26a0da commit 7ee16b5
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 11 deletions.
6 changes: 3 additions & 3 deletions core/primitives/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use crate::hash::{hash, CryptoHash};
use crate::hash::{hash as sha256, CryptoHash};

pub struct ContractCode {
pub code: Vec<u8>,
pub hash: CryptoHash,
}

impl ContractCode {
pub fn new(code: Vec<u8>) -> ContractCode {
let hash = hash(&code);
pub fn new(code: Vec<u8>, hash: Option<CryptoHash>) -> ContractCode {
let hash = hash.unwrap_or_else(|| sha256(&code));
ContractCode { code, hash }
}

Expand Down
3 changes: 2 additions & 1 deletion core/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,11 @@ pub fn set_code(state_update: &mut TrieUpdate, account_id: AccountId, code: &Con
pub fn get_code(
state_update: &TrieUpdate,
account_id: &AccountId,
code_hash: Option<CryptoHash>,
) -> Result<Option<ContractCode>, StorageError> {
state_update
.get(&TrieKey::ContractCode { account_id: account_id.clone() })
.map(|opt| opt.map(|code| ContractCode::new(code.to_vec())))
.map(|opt| opt.map(|code| ContractCode::new(code, code_hash)))
}

/// Removes account, code and all access keys associated to it.
Expand Down
2 changes: 1 addition & 1 deletion genesis-tools/genesis-populate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl GenesisBuilder {
);
records.push(access_key_record);
if let Some(wasm_binary) = self.additional_accounts_code.as_ref() {
let code = ContractCode::new(wasm_binary.clone());
let code = ContractCode::new(wasm_binary.clone(), None);
set_code(&mut state_update, account_id.clone(), &code);
let contract_record = StateRecord::Contract { account_id, code: wasm_binary.clone() };
records.push(contract_record);
Expand Down
6 changes: 3 additions & 3 deletions runtime/runtime/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub(crate) fn get_code_with_cache(
) -> Result<Option<Arc<ContractCode>>, StorageError> {
debug!(target:"runtime", "Calling the contract at account {}", account_id);
let code_hash = account.code_hash;
let code = || get_code(state_update, account_id);
let code = || get_code(state_update, account_id, Some(code_hash));
crate::cache::get_code(code_hash, code)
}

Expand Down Expand Up @@ -343,8 +343,8 @@ pub(crate) fn action_deploy_contract(
account_id: &AccountId,
deploy_contract: &DeployContractAction,
) -> Result<(), StorageError> {
let code = ContractCode::new(deploy_contract.code.clone());
let prev_code = get_code(state_update, account_id)?;
let code = ContractCode::new(deploy_contract.code.clone(), None);
let prev_code = get_code(state_update, account_id, Some(account.code_hash))?;
let prev_code_length = prev_code.map(|code| code.code.len() as u64).unwrap_or_default();
account.storage_usage =
account.storage_usage.checked_sub(prev_code_length).ok_or_else(|| {
Expand Down
3 changes: 2 additions & 1 deletion runtime/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,8 @@ impl Runtime {
}
StateRecord::Contract { account_id, code } => {
let acc = get_account(&state_update, &account_id).expect("Failed to read state").expect("Code state record should be preceded by the corresponding account record");
let code = ContractCode::new(code);
// Recompute contract code hash.
let code = ContractCode::new(code, None);
set_code(&mut state_update, account_id, &code);
assert_eq!(code.get_hash(), acc.code_hash);
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/runtime/tests/test_regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn template_test(transaction_type: TransactionType, db_type: DataBaseType, expec
&AccessKey::full_access(),
);
records.push(access_key_record);
let code = ContractCode::new(wasm_binary.to_vec());
let code = ContractCode::new(wasm_binary.to_vec(), Some(code_hash));
set_code(&mut state_update, account_id.clone(), &code);
let contract_record = StateRecord::Contract {
account_id: account_id.clone(),
Expand Down
2 changes: 1 addition & 1 deletion test-utils/testlib/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ pub fn create_nodes_from_seeds(seeds: Vec<String>) -> Vec<NodeConfig> {
{
if *record_account_id == seed {
is_account_record_found = true;
account.code_hash = ContractCode::new(code.clone()).get_hash();
account.code_hash = ContractCode::new(code.clone(), None).get_hash();
}
}
}
Expand Down

0 comments on commit 7ee16b5

Please sign in to comment.