Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

Commit

Permalink
Merge pull request from GHSA-qmc9-4hg8-q3p8
Browse files Browse the repository at this point in the history
* fix: CALLCODE and DELEGATECALL should skip transfer

There is only one place calling `handle_transfer`, so we could put this
`special_call` check in the caller side to save cycles

* test: fix delegatecall test case
  • Loading branch information
Flouse authored May 6, 2022
1 parent 8688af8 commit 972521f
Show file tree
Hide file tree
Showing 13 changed files with 302 additions and 30 deletions.
21 changes: 15 additions & 6 deletions c/polyjuice.h
Original file line number Diff line number Diff line change
Expand Up @@ -1131,12 +1131,21 @@ int handle_message(gw_context_t* ctx,
}
}

/* Handle transfer logic.
NOTE: MUST do this before vm.execute and after to_id finalized */
bool to_address_is_eoa = !to_address_exists || (to_address_exists && code_size == 0);
ret = handle_transfer(ctx, &msg, to_address_is_eoa);
if (ret != 0) {
return ret;
/**
* Handle transfer logic
*
* NOTE:
* 1. MUST do this before vm.execute and after to_id finalized
* 2. CALLCODE/DELEGATECALL should skip `handle_transfer`, otherwise
* `value transfer` of CALLCODE/DELEGATECALL will be executed twice
*/
if (!is_special_call(msg.kind)) {
bool to_address_is_eoa = !to_address_exists
|| (to_address_exists && code_size == 0);
ret = handle_transfer(ctx, &msg, to_address_is_eoa);
if (ret != 0) {
return ret;
}
}

debug_print_int("[handle_message] msg.kind", msg.kind);
Expand Down
2 changes: 1 addition & 1 deletion c/polyjuice_globals.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef POLYJUICE_GLOBALS_H
#define POLYJUICE_GLOBALS_H

#define POLYJUICE_VERSION "v1.1.3-beta"
#define POLYJUICE_VERSION "v1.1.4-beta"

#define ETH_ADDRESS_LEN 20

Expand Down
26 changes: 13 additions & 13 deletions polyjuice-tests/Cargo.lock

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

157 changes: 157 additions & 0 deletions polyjuice-tests/src/test_cases/beacon_proxy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
use crate::helper::{self, MockContractInfo, L2TX_MAX_CYCLES};
use gw_common::{
builtins::{CKB_SUDT_ACCOUNT_ID, ETH_REGISTRY_ACCOUNT_ID},
registry_address::RegistryAddress,
state::State,
};
use gw_generator::traits::StateExt;
use gw_store::{chain_view::ChainView, traits::chain_store::ChainStore};
use gw_types::{
bytes::Bytes,
packed::RawL2Transaction,
prelude::{Builder, Entity, Pack}, U256,
};

const CONTRACT_CODE: &str = include_str!("./evm-contracts/BeaconProxy.bin");
const EVMC_SUCCESS: i8 = 0;

#[test]
fn test_beacon_proxy() {
let (store, mut state, generator) = helper::setup();
let block_producer = helper::create_block_producer(&mut state);
let mut block_number = 0;

// init accounts
let from_eth_address = [1u8; 20];
let (from_id, _from_script_hash) =
helper::create_eth_eoa_account(&mut state, &from_eth_address, U256::from(2000000u64));

// deploy payableInitializationBugContractTest contract
let run_result = helper::deploy(
&generator,
&store,
&mut state,
helper::CREATOR_ACCOUNT_ID,
from_id,
CONTRACT_CODE,
80000,
0,
block_producer.to_owned(),
block_number,
);
assert_eq!(run_result.exit_code, EVMC_SUCCESS);
let contract = MockContractInfo::create(&from_eth_address, 0);
let contract_account_id = state
.get_account_id_by_script_hash(&contract.script_hash)
.unwrap()
.expect("get_account_id_by_script_hash");

// invoke payableInitializationBugContractTest.init() -> 0xe1c7392a
block_number += 1;
let block_info = helper::new_block_info(block_producer.to_owned(), block_number, block_number);
let input = hex::decode("e1c7392a").expect("init() method ID");
let args = helper::PolyjuiceArgsBuilder::default()
.gas_limit(1829630)
.gas_price(1)
.value(0)
.input(&input)
.build();
let raw_tx = RawL2Transaction::new_builder()
.from_id(from_id.pack())
.to_id(contract_account_id.pack())
.args(Bytes::from(args).pack())
.build();
let db = store.begin_transaction();
let tip_block_hash = store.get_tip_block_hash().unwrap();
let run_result = generator
.execute_transaction(
&ChainView::new(&db, tip_block_hash),
&state,
&block_info,
&raw_tx,
L2TX_MAX_CYCLES,
None,
)
.expect("invode initializaion");
assert_eq!(run_result.exit_code, EVMC_SUCCESS);
state.apply_run_result(&run_result).expect("update state");

state
.mint_sudt(CKB_SUDT_ACCOUNT_ID, &contract.reg_addr, U256::from(200u64))
.unwrap();

// deployBeaconProxy(
// upgradeable_beacon_addr,
// ethers.utils.arrayify("0xe79f5bee0000000000000000000000000000000000000000000000000000000000000037"),
// { value: 110 })
block_number += 1;
let block_info = helper::new_block_info(block_producer.to_owned(), block_number, block_number);
let input = hex::decode("dc3bdc5500000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000024e79f5bee000000000000000000000000000000000000000000000000000000000000003700000000000000000000000000000000000000000000000000000000").expect("deployBeaconProxy(bytes)");
const DEPOLOY_MES_VALUE: u128 = 17;
let args = helper::PolyjuiceArgsBuilder::default()
.gas_limit(447270)
.gas_price(1)
.value(DEPOLOY_MES_VALUE)
.input(&input)
.build();
let raw_tx = RawL2Transaction::new_builder()
.from_id(from_id.pack())
.to_id(contract_account_id.pack())
.args(Bytes::from(args).pack())
.build();
let db = store.begin_transaction();
let tip_block_hash = store.get_tip_block_hash().unwrap();
let run_result = generator
.execute_transaction(
&ChainView::new(&db, tip_block_hash),
&state,
&block_info,
&raw_tx,
L2TX_MAX_CYCLES,
None,
)
.expect("Call deployBeaconProxy");
assert_eq!(run_result.exit_code, EVMC_SUCCESS);
state.apply_run_result(&run_result).expect("update state");

// get BeaconProxy public bpx and check it's balance
block_number += 1;
let block_info = helper::new_block_info(block_producer, block_number, block_number);
let input = hex::decode("c6662850").expect("bpx.get() method id");
let args = helper::PolyjuiceArgsBuilder::default()
.gas_limit(23778)
.gas_price(1)
.value(0)
.input(&input)
.build();
let raw_tx = RawL2Transaction::new_builder()
.from_id(from_id.pack())
.to_id(contract_account_id.pack())
.args(Bytes::from(args).pack())
.build();
let db = store.begin_transaction();
let tip_block_hash = store.get_tip_block_hash().unwrap();
let run_result = generator
.execute_transaction(
&ChainView::new(&db, tip_block_hash),
&state,
&block_info,
&raw_tx,
L2TX_MAX_CYCLES,
None,
)
.expect("get BeaconProxy contract address");
assert_eq!(run_result.exit_code, EVMC_SUCCESS);
state.apply_run_result(&run_result).expect("update state");
assert_eq!(run_result.return_data.len(), 32);
let beacon_proxy_ethabi_addr = &run_result.return_data[12..];
let beacon_reg_addr =
RegistryAddress::new(ETH_REGISTRY_ACCOUNT_ID, beacon_proxy_ethabi_addr.to_vec());
assert_eq!(
state
.get_sudt_balance(CKB_SUDT_ACCOUNT_ID, &beacon_reg_addr)
.unwrap(),
U256::from(DEPOLOY_MES_VALUE),
"check the balance of BeaconProxy contract"
);
}
23 changes: 15 additions & 8 deletions polyjuice-tests/src/test_cases/delegatecall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::helper::{
new_contract_account_script_with_nonce, setup, simple_storage_get, MockContractInfo,
PolyjuiceArgsBuilder, CREATOR_ACCOUNT_ID, L2TX_MAX_CYCLES,
};
use gw_common::state::State;
use gw_common::{state::State, builtins::CKB_SUDT_ACCOUNT_ID};
use gw_generator::traits::StateExt;
use gw_store::chain_view::ChainView;
use gw_store::traits::chain_store::ChainStore;
Expand All @@ -18,7 +18,7 @@ const INIT_CODE: &str = include_str!("./evm-contracts/DelegateCall.bin");
#[test]
fn test_delegatecall() {
let (store, mut state, generator) = setup();
let block_producer_id = helper::create_block_producer(&mut state);
let block_producer = helper::create_block_producer(&mut state);

let from_eth_address = [1u8; 20];
let (from_id, _from_script_hash) =
Expand All @@ -35,7 +35,7 @@ fn test_delegatecall() {
SS_INIT_CODE,
122000,
0,
block_producer_id.clone(),
block_producer.clone(),
block_number,
);
let ss_account_script = new_contract_account_script_with_nonce(&from_eth_address, 0);
Expand All @@ -55,7 +55,7 @@ fn test_delegatecall() {
INIT_CODE,
122000,
0,
block_producer_id.clone(),
block_producer.clone(),
block_number,
);
// [Deploy DelegateCall] used cycles: 753698 < 760K
Expand All @@ -74,13 +74,15 @@ fn test_delegatecall() {
assert_eq!(state.get_nonce(from_id).unwrap(), 2);
assert_eq!(state.get_nonce(ss_account_id).unwrap(), 0);
assert_eq!(state.get_nonce(delegate_contract_id).unwrap(), 0);

/*
* In a delegatecall, only the code of the given address is used, all other aspects (storage,
* balance, …) are taken from the current contract.
* The purpose of delegatecall is to use library code which is stored in another contract.
* The user has to ensure that the layout of storage in both contracts is suitable for
* delegatecall to be used.
*/
const MSG_VALUE: u128 = 17;
for (fn_sighash, expected_return_value) in [
// DelegateCall.set(address, uint) => used cycles: 1002251
(
Expand All @@ -101,7 +103,7 @@ fn test_delegatecall() {
.iter()
{
block_number += 1;
let block_info = new_block_info(block_producer_id.clone(), block_number, block_number);
let block_info = new_block_info(block_producer.clone(), block_number, block_number);
let input = hex::decode(format!(
"{}{}{}",
fn_sighash,
Expand All @@ -112,7 +114,7 @@ fn test_delegatecall() {
let args = PolyjuiceArgsBuilder::default()
.gas_limit(200000)
.gas_price(1)
.value(0)
.value(MSG_VALUE)
.input(&input)
.build();
let raw_tx = RawL2Transaction::new_builder()
Expand Down Expand Up @@ -153,7 +155,11 @@ fn test_delegatecall() {
hex::decode(expected_return_value).unwrap()
);
}

// check the balance of DelegateCall contract
let delegate_contract_balance = state
.get_sudt_balance(CKB_SUDT_ACCOUNT_ID, &delegate_contract.reg_addr)
.unwrap();
assert_eq!(delegate_contract_balance, gw_types::U256::from(MSG_VALUE * 3));
assert_eq!(state.get_nonce(from_id).unwrap(), 5);
assert_eq!(state.get_nonce(ss_account_id).unwrap(), 0);
assert_eq!(state.get_nonce(delegate_contract_id).unwrap(), 0);
Expand All @@ -168,6 +174,7 @@ fn test_delegatecall() {
);
assert_eq!(
run_result.return_data,
hex::decode("000000000000000000000000000000000000000000000000000000000000007b").unwrap()
hex::decode("000000000000000000000000000000000000000000000000000000000000007b").unwrap(),
"The storedData in SimepleStorage contract won't be changed."
);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-License-Identifier: SEE LICENSE IN LICENSE
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

contract AddressType {
Expand Down

Large diffs are not rendered by default.

Loading

0 comments on commit 972521f

Please sign in to comment.