From 90d5330a4a2208c1497defad1b0b6b12b09b554c Mon Sep 17 00:00:00 2001 From: Flouse Date: Wed, 11 May 2022 02:18:13 +0800 Subject: [PATCH 1/3] Fix calculate_fee => uint256_t --- Makefile | 5 +- c/polyjuice.h | 18 ++-- c/polyjuice_utils.h | 30 ++++++ c/tests/test_calc_fee.c | 48 ++++++++++ polyjuice-tests/Cargo.lock | 2 +- polyjuice-tests/Cargo.toml | 2 +- polyjuice-tests/src/helper.rs | 2 +- .../src/test_cases/beacon_proxy.rs | 3 +- .../src/test_cases/delegatecall.rs | 7 +- polyjuice-tests/src/test_cases/error.rs | 2 +- polyjuice-tests/src/test_cases/mod.rs | 1 + .../src/test_cases/simple_transfer.rs | 2 +- polyjuice-tests/src/test_cases/utils.rs | 94 +++++++++++++++++++ 13 files changed, 196 insertions(+), 20 deletions(-) create mode 100644 c/tests/test_calc_fee.c create mode 100644 polyjuice-tests/src/test_cases/utils.rs diff --git a/Makefile b/Makefile index 943191a7..5e0737e9 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ BUILDER_DOCKER := nervos/ckb-riscv-gnu-toolchain@sha256:7b168b4b109a0f741078a71b # TODO: update to nervos/ckb-riscv-gnu-toolchain:bionic-20211214 => need more tests all: build/blockchain.h build/godwoken.h \ - build/test_contracts build/test_rlp build/test_ripemd160 \ + build/test_contracts build/test_rlp build/test_ripemd160 build/test_calc_fee \ build/generator build/validator \ build/generator_log build/validator_log @@ -150,6 +150,9 @@ build/test_rlp: c/tests/test_rlp.c $(VALIDATOR_DEPS) $(OBJCOPY) --strip-debug --strip-all $@ cd $(SECP_DIR) && (git apply -R workaround-fix-g++-linking.patch || true) && cd - # revert patch +build/test_calc_fee: c/tests/test_calc_fee.c $(VALIDATOR_DEPS) + $(CXX) $(CFLAGS) $(LDFLAGS) -Ibuild -o $@ c/tests/test_calc_fee.c $(ALL_OBJS) + build/test_ripemd160: c/ripemd160/test_ripemd160.c c/ripemd160/ripemd160.h c/ripemd160/memzero.h $(ALL_OBJS) $(CXX) $(CFLAGS) $(LDFLAGS) -Ibuild -o $@ c/ripemd160/test_ripemd160.c $(ALL_OBJS) riscv64-unknown-elf-run build/test_ripemd160 diff --git a/c/polyjuice.h b/c/polyjuice.h index 54db78ac..d779f567 100644 --- a/c/polyjuice.h +++ b/c/polyjuice.h @@ -1350,6 +1350,10 @@ int run_polyjuice() { return clean_evmc_result_and_return(&res, ret_handle_message); } + uint32_t used_memory; + memcpy(&used_memory, res.padding, sizeof(uint32_t)); + debug_print_int("[run_polyjuice] used_memory(Bytes)", used_memory); + /* Handle transaction fee */ if (res.gas_left < 0) { ckb_debug("gas not enough"); @@ -1361,21 +1365,12 @@ int run_polyjuice() { ckb_debug("unreachable!"); return clean_evmc_result_and_return(&res, -1); } - uint128_t fee = gas_price * (uint128_t)gas_used; + debug_print_int("gas limit", msg.gas); debug_print_int("gas left", res.gas_left); - debug_print_int("gas price", gas_price); - debug_print_int("fee", fee); - - uint32_t used_memory; - memcpy(&used_memory, res.padding, sizeof(uint32_t)); - debug_print_int("[run_polyjuice] used_memory(Bytes)", used_memory); + uint256_t fee_u256 = calculate_fee(gas_price, gas_used); gw_reg_addr_t sender_addr = new_reg_addr(msg.sender.bytes); - - uint256_t fee_u256 = {0}; - memcpy((uint8_t *)(&fee_u256), (uint8_t*)(&fee), 16); - ret = sudt_pay_fee(&context, g_sudt_id, /* g_sudt_id must already exists */ sender_addr, fee_u256); if (ret != 0) { @@ -1383,6 +1378,7 @@ int run_polyjuice() { return clean_evmc_result_and_return(&res, ret); } + // TODO: duplicate call? // call the SYS_PAY_FEE syscall to record the fee // NOTICE: this function do not actually execute the transfer of assets ret = sys_pay_fee(&context, sender_addr, g_sudt_id, fee_u256); diff --git a/c/polyjuice_utils.h b/c/polyjuice_utils.h index 7c413de9..e46e239d 100644 --- a/c/polyjuice_utils.h +++ b/c/polyjuice_utils.h @@ -249,6 +249,36 @@ int parse_u256(const uint8_t data_be[32], uint256_t *value) { return parse_integer(data_be, (uint8_t *)value, sizeof(uint256_t)); } +uint128_t hi(uint128_t x) { + return x >> 64; +} + +uint128_t lo(uint128_t x) { + return 0xFFFFFFFFFFFFFFFF & x; +} + +/** + * @brief calculate fee => gas_price * gas_used + * + * Multiplication Algorithm + * https://en.wikipedia.org/wiki/Multiplication_algorithm + * + * @param gas_price msg.gas_price + * @param gas_used = msg.gas_limit - res.gas_left + * @return uint256_t + */ +uint256_t calculate_fee(uint128_t gas_price, uint64_t gas_used) { + uint128_t price_high = hi(gas_price); + uint128_t price_low = lo(gas_price); + uint128_t fee_low = price_low * gas_used; + uint128_t fee_high = hi(fee_low) + price_high * gas_used; + + uint256_t fee_u256 = {0}; + _gw_fast_memcpy((uint8_t *)(&fee_u256), (uint8_t*)(&fee_low), 8); + _gw_fast_memcpy((uint8_t *)(&fee_u256) + 8, (uint8_t*)(&fee_high), 16); + return fee_u256; +} + /* serialize uint64_t to big endian byte32 */ void put_u64(uint64_t value, uint8_t *output) { uint8_t *value_le = (uint8_t *)(&value); diff --git a/c/tests/test_calc_fee.c b/c/tests/test_calc_fee.c new file mode 100644 index 00000000..6172f4e0 --- /dev/null +++ b/c/tests/test_calc_fee.c @@ -0,0 +1,48 @@ +#include "test_utils.h" +#include "gw_def.h" +#include "generator_utils.h" +#include "../polyjuice_utils.h" +#include + +void test(uint128_t gas_price, uint64_t gas_used, uint256_t expected_fee) { + uint256_t result = calculate_fee(gas_price, gas_used); + assert(gw_uint256_cmp(result, expected_fee) == GW_UINT256_EQUAL); +} + +int main() { + uint256_t expected_result = {0}; + gw_uint256_cmp(expected_result, expected_result); + test(0, 0, expected_result); + test(0, 1, expected_result); + test(1, 0, expected_result); + + gw_uint256_one(&expected_result); + test(1, 1, expected_result); + + uint128_t gas_price = 11; + expected_result.array[0] = 22; + test(gas_price, 2, expected_result); + + gas_price = 0xfedbca9876543210ULL; + expected_result.array[0] = 0x76543210; + expected_result.array[1] = 0xfedbca98; + test(gas_price, 1, expected_result); + test(gas_price, 2, {0xECA86420UL, 0xFDB79530UL, 0x1UL}); + + gas_price = ((uint128_t)0xF0F0F0F0F0F0F0F0 << 64) | 0xF0F0F0F0F0F0F0F0; + gw_uint256_zero(&expected_result); + test(gas_price, 0, expected_result); + test(gas_price, 1, {0xF0F0F0F0UL, 0xF0F0F0F0UL, 0xF0F0F0F0UL, 0xF0F0F0F0UL}); + + uint64_t gas_used = 0xaaaaaaaaaaaaaaaaULL; + test(gas_price, gas_used, {0x5f5f5f60, 0x5f5f5f5f, 0xffffffff, 0xffffffff, + 0xA0A0A09F, 0xA0A0A0A0, 0x00000000, 0x00000000}); + + const uint64_t MAX_UINT64 = 0xFFFFFFFFFFFFFFFF; + gas_used = MAX_UINT64; + gas_price = ((uint128_t)MAX_UINT64 << 64) | MAX_UINT64; + test(gas_price, gas_used, {0x00000001, 0x00000000, 0xffffffff, 0xffffffff, + 0xFFFFFFFE, 0xFFFFFFFF, 0x00000000, 0x00000000}); + + return 0; +} diff --git a/polyjuice-tests/Cargo.lock b/polyjuice-tests/Cargo.lock index 75a6dc6a..de00a017 100644 --- a/polyjuice-tests/Cargo.lock +++ b/polyjuice-tests/Cargo.lock @@ -1425,7 +1425,7 @@ checksum = "b4596b6d070b27117e987119b4dac604f3c58cfb0b191112e24771b2faeac1a6" [[package]] name = "polyjuice-tests" -version = "1.1.0" +version = "1.1.5" dependencies = [ "ckb-vm", "env_logger", diff --git a/polyjuice-tests/Cargo.toml b/polyjuice-tests/Cargo.toml index 652f46e1..9b3346b7 100644 --- a/polyjuice-tests/Cargo.toml +++ b/polyjuice-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "polyjuice-tests" -version = "1.1.0" +version = "1.1.5" authors = ["Linfeng Qian "] edition = "2018" diff --git a/polyjuice-tests/src/helper.rs b/polyjuice-tests/src/helper.rs index 1ac03035..25ac536a 100644 --- a/polyjuice-tests/src/helper.rs +++ b/polyjuice-tests/src/helper.rs @@ -154,7 +154,7 @@ fn parse_sudt_log_data(data: &[u8]) -> (RegistryAddress, RegistryAddress, U256) let mut u256_bytes = [0u8; 32]; u256_bytes.copy_from_slice(&data[56..56 + 32]); - let amount = U256::from_little_endian(&mut u256_bytes); + let amount = U256::from_little_endian(&u256_bytes); (from_addr, to_addr, amount) } diff --git a/polyjuice-tests/src/test_cases/beacon_proxy.rs b/polyjuice-tests/src/test_cases/beacon_proxy.rs index 12acf3f3..7e9d3ea2 100644 --- a/polyjuice-tests/src/test_cases/beacon_proxy.rs +++ b/polyjuice-tests/src/test_cases/beacon_proxy.rs @@ -9,7 +9,8 @@ use gw_store::{chain_view::ChainView, traits::chain_store::ChainStore}; use gw_types::{ bytes::Bytes, packed::RawL2Transaction, - prelude::{Builder, Entity, Pack}, U256, + prelude::{Builder, Entity, Pack}, + U256, }; const CONTRACT_CODE: &str = include_str!("./evm-contracts/BeaconProxy.bin"); diff --git a/polyjuice-tests/src/test_cases/delegatecall.rs b/polyjuice-tests/src/test_cases/delegatecall.rs index d917f123..0976a16f 100644 --- a/polyjuice-tests/src/test_cases/delegatecall.rs +++ b/polyjuice-tests/src/test_cases/delegatecall.rs @@ -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, builtins::CKB_SUDT_ACCOUNT_ID}; +use gw_common::{builtins::CKB_SUDT_ACCOUNT_ID, state::State}; use gw_generator::traits::StateExt; use gw_store::chain_view::ChainView; use gw_store::traits::chain_store::ChainStore; @@ -159,7 +159,10 @@ fn test_delegatecall() { 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!( + 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); diff --git a/polyjuice-tests/src/test_cases/error.rs b/polyjuice-tests/src/test_cases/error.rs index c8b6c6d9..1c8b57e6 100644 --- a/polyjuice-tests/src/test_cases/error.rs +++ b/polyjuice-tests/src/test_cases/error.rs @@ -221,7 +221,7 @@ fn test_error_handling() { // testRevertMsg("test revert message") -> 0x5729f42c000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000137465737420726576657274206d65737361676500000000000000000000000000 block_number += 1; - let block_info = helper::new_block_info(block_producer.to_owned(), block_number, block_number); + let block_info = helper::new_block_info(block_producer, block_number, block_number); let input = hex::decode("5729f42c000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000137465737420726576657274206d65737361676500000000000000000000000000") .expect("decode the function selector and arg of testRevertMsg('test revert message')"); diff --git a/polyjuice-tests/src/test_cases/mod.rs b/polyjuice-tests/src/test_cases/mod.rs index a439669b..161d8e99 100644 --- a/polyjuice-tests/src/test_cases/mod.rs +++ b/polyjuice-tests/src/test_cases/mod.rs @@ -30,3 +30,4 @@ pub(crate) mod sudt_erc20_proxy; mod beacon_proxy; mod error; mod eth_addr_reg; +mod utils; diff --git a/polyjuice-tests/src/test_cases/simple_transfer.rs b/polyjuice-tests/src/test_cases/simple_transfer.rs index 1b0c7935..96caf98b 100644 --- a/polyjuice-tests/src/test_cases/simple_transfer.rs +++ b/polyjuice-tests/src/test_cases/simple_transfer.rs @@ -26,7 +26,7 @@ fn test_simple_transfer() { let mint_balance = U256::from(400000u128); let from_eth_address = [1u8; 20]; let (from_id, _from_script_hash) = - helper::create_eth_eoa_account(&mut state, &from_eth_address, mint_balance.into()); + helper::create_eth_eoa_account(&mut state, &from_eth_address, mint_balance); let from_addr = RegistryAddress::new(ETH_REGISTRY_ACCOUNT_ID, from_eth_address.to_vec()); let target_eth_addr = [2u8; 20]; diff --git a/polyjuice-tests/src/test_cases/utils.rs b/polyjuice-tests/src/test_cases/utils.rs new file mode 100644 index 00000000..39a99c09 --- /dev/null +++ b/polyjuice-tests/src/test_cases/utils.rs @@ -0,0 +1,94 @@ +use crate::helper::L2TX_MAX_CYCLES; +use ckb_vm::{ + machine::asm::{AsmCoreMachine, AsmMachine}, + memory::Memory, + registers::{A0, A7}, + DefaultMachineBuilder, Error as VMError, Register, SupportMachine, Syscalls, +}; +use gw_types::bytes::Bytes; + +const BINARY: &[u8] = include_bytes!("../../../build/test_calc_fee"); +const DEBUG_PRINT_SYSCALL_NUMBER: u64 = 2177; + +pub struct L2Syscalls; + +impl Syscalls for L2Syscalls { + fn initialize(&mut self, _machine: &mut Mac) -> Result<(), VMError> { + Ok(()) + } + + fn ecall(&mut self, machine: &mut Mac) -> Result { + let code = machine.registers()[A7].to_u64(); + if code != DEBUG_PRINT_SYSCALL_NUMBER { + println!("code: {}", code); + } + match code { + DEBUG_PRINT_SYSCALL_NUMBER => { + self.output_debug(machine)?; + Ok(true) + } + _ => Ok(false), + } + } +} + +impl L2Syscalls { + fn output_debug(&self, machine: &mut Mac) -> Result<(), VMError> { + let mut addr = machine.registers()[A0].to_u64(); + let mut buffer = Vec::new(); + + loop { + let byte = machine + .memory_mut() + .load8(&Mac::REG::from_u64(addr))? + .to_u8(); + if byte == 0 { + break; + } + buffer.push(byte); + addr += 1; + } + + let s = String::from_utf8(buffer).map_err(|_| VMError::ParseError)?; + println!("[debug]: {}", s); + Ok(()) + } +} + +// TODO: refactor +struct AsmCoreMachineParams { + pub vm_isa: u8, + pub vm_version: u32, +} + +impl AsmCoreMachineParams { + pub fn with_version(vm_version: u32) -> Result { + if vm_version == 0 { + Ok(AsmCoreMachineParams { + vm_isa: ckb_vm::ISA_IMC, + vm_version: ckb_vm::machine::VERSION0, + }) + } else if vm_version == 1 { + Ok(AsmCoreMachineParams { + vm_isa: ckb_vm::ISA_IMC | ckb_vm::ISA_B | ckb_vm::ISA_MOP, + vm_version: ckb_vm::machine::VERSION1, + }) + } else { + Err(VMError::InvalidVersion) + } + } +} + +#[test] +fn test_calc_fee() { + let binary: Bytes = BINARY.to_vec().into(); + + let params = AsmCoreMachineParams::with_version(1).unwrap(); + let core_machine = AsmCoreMachine::new(params.vm_isa, params.vm_version, L2TX_MAX_CYCLES); + let machine_builder = DefaultMachineBuilder::new(core_machine).syscall(Box::new(L2Syscalls)); + let mut machine = AsmMachine::new(machine_builder.build(), None); + + machine.load_program(&binary, &[]).unwrap(); + let code = machine.run().unwrap(); + assert_eq!(code, 0); +} From e26155eeaa140d2f2c1b9c65a59c7c6859a5a87b Mon Sep 17 00:00:00 2001 From: Flouse Date: Wed, 11 May 2022 10:33:35 +0800 Subject: [PATCH 2/3] Remove duplicated pay_fee_syscall `sudt_pay_fee` already call SYS_PAY_FEE see: https://github.com/nervosnetwork/godwoken-scripts/blob/5986f71/c/sudt_utils.h#L240 --- c/polyjuice.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/c/polyjuice.h b/c/polyjuice.h index d779f567..1b808cf8 100644 --- a/c/polyjuice.h +++ b/c/polyjuice.h @@ -1378,15 +1378,6 @@ int run_polyjuice() { return clean_evmc_result_and_return(&res, ret); } - // TODO: duplicate call? - // call the SYS_PAY_FEE syscall to record the fee - // NOTICE: this function do not actually execute the transfer of assets - ret = sys_pay_fee(&context, sender_addr, g_sudt_id, fee_u256); - if (ret != 0) { - debug_print_int("[run_polyjuice] Record fee payment failed", ret); - return clean_evmc_result_and_return(&res, ret); - } - ckb_debug("[run_polyjuice] finalize"); ret = gw_finalize(&context); if (ret != 0) { From b66a77b88aa0dcc47096264fc267b0150ac81ad5 Mon Sep 17 00:00:00 2001 From: Flouse Date: Wed, 11 May 2022 11:29:57 +0800 Subject: [PATCH 3/3] Bump POLYJUICE_VERSION to "v1.1.5-beta" --- c/polyjuice_globals.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/polyjuice_globals.h b/c/polyjuice_globals.h index 78e46310..6f44c9cd 100644 --- a/c/polyjuice_globals.h +++ b/c/polyjuice_globals.h @@ -1,7 +1,7 @@ #ifndef POLYJUICE_GLOBALS_H #define POLYJUICE_GLOBALS_H -#define POLYJUICE_VERSION "v1.1.4-beta" +#define POLYJUICE_VERSION "v1.1.5-beta" #define ETH_ADDRESS_LEN 20