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

fix(runtime) - Mitigate the receipt size limit bug #12633

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
85 changes: 85 additions & 0 deletions integration-tests/src/test_loop/tests/max_receipt_size.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use assert_matches::assert_matches;
use near_async::time::Duration;
use near_o11y::testonly::init_test_logger;
use near_primitives::action::{Action, FunctionCallAction};
use near_primitives::errors::{
ActionErrorKind, InvalidTxError, ReceiptValidationError, TxExecutionError,
};
use near_primitives::hash::CryptoHash;
use near_primitives::receipt::{ActionReceipt, Receipt, ReceiptEnum, ReceiptV0};
use near_primitives::test_utils::create_user_test_signer;
use near_primitives::transaction::SignedTransaction;
use near_primitives::types::AccountId;
Expand Down Expand Up @@ -116,3 +119,85 @@ fn slow_test_max_receipt_size() {

env.shutdown_and_drain_remaining_events(Duration::seconds(20));
}

// A function call will generate a new receipt. Size of this receipt will be equal to
// `max_receipt_size`, it'll pass validation, but then `output_data_receivers` will be modified and
// the receipt's size will go above max_receipt_size. The receipt should be rejected, but currently
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you assert that this condition is actually triggered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an assert that there really was a receipt with size above max_receipt_size.

// isn't because of a bug (See https://github.com/near/nearcore/issues/12606)
// Runtime shouldn't die when it encounters a receipt with size above `max_receipt_size`.
#[test]
fn test_max_receipt_size_promise_return() {
init_test_logger();
let mut env: TestLoopEnv = standard_setup_1();

let account: AccountId = "account0".parse().unwrap();
let account_signer = &create_user_test_signer(&account).into();

// Deploy the test contract
let deploy_contract_tx = SignedTransaction::deploy_contract(
101,
&account,
near_test_contracts::rs_contract().into(),
&account_signer,
get_shared_block_hash(&env.datas, &env.test_loop),
);
run_tx(&mut env.test_loop, deploy_contract_tx, &env.datas, Duration::seconds(5));

// User calls a contract method
// Contract method creates a DAG with two promises: [A -then-> B]
// When promise A is executed, it creates a third promise - `C` and does a `promise_return`.
// The DAG changes to: [C ->then-> B]
// The receipt for promise C is a maximum size receipt.
// Adding the `output_data_receivers` to C's receipt makes it go over the size limit.
let base_receipt_template = Receipt::V0(ReceiptV0 {
predecessor_id: account.clone(),
receiver_id: account.clone(),
receipt_id: CryptoHash::default(),
receipt: ReceiptEnum::Action(ActionReceipt {
signer_id: account.clone(),
signer_public_key: account_signer.public_key().into(),
gas_price: 0,
output_data_receivers: vec![],
input_data_ids: vec![],
actions: vec![Action::FunctionCall(Box::new(FunctionCallAction {
method_name: "noop".into(),
args: vec![],
gas: 0,
deposit: 0,
}))],
}),
});
let base_receipt_size = borsh::object_length(&base_receipt_template).unwrap();
let max_receipt_size = 4_194_304;
let args_size = max_receipt_size - base_receipt_size;

// Call the contract
let large_receipt_tx = SignedTransaction::call(
102,
account.clone(),
account.clone(),
&account_signer,
0,
"max_receipt_size_promise_return_method1".into(),
format!("{{\"args_size\": {}}}", args_size).into(),
300 * TGAS,
get_shared_block_hash(&env.datas, &env.test_loop),
);
run_tx(&mut env.test_loop, large_receipt_tx, &env.datas, Duration::seconds(5));

// Make sure that the last promise in the DAG was called
let assert_test_completed = SignedTransaction::call(
103,
account.clone(),
account,
&account_signer,
0,
"assert_test_completed".into(),
"".into(),
300 * TGAS,
get_shared_block_hash(&env.datas, &env.test_loop),
);
run_tx(&mut env.test_loop, assert_test_completed, &env.datas, Duration::seconds(5));

env.shutdown_and_drain_remaining_events(Duration::seconds(20));
}
107 changes: 107 additions & 0 deletions runtime/near-test-contracts/test-contract-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ extern "C" {
fn burn_gas(gas: u64);
}

const TGAS: u64 = 1_000_000_000_000;

macro_rules! ext_test {
($export_func:ident, $call_ext:expr) => {
#[unsafe(no_mangle)]
Expand Down Expand Up @@ -1676,3 +1678,108 @@ pub unsafe fn do_function_call_with_args_of_size() {
gas_weight,
);
}

/// Used by the `max_receipt_size_promise_return` test.
/// Create promise DAG:
/// A[self.max_receipt_size_promise_return_method2()] -then-> B[self.mark_test_completed()]
#[no_mangle]
pub unsafe fn max_receipt_size_promise_return_method1() {
input(0);
let args = vec![0u8; register_len(0) as usize];
read_register(0, args.as_ptr() as u64);

current_account_id(0);
let current_account = vec![0u8; register_len(0) as usize];
read_register(0, current_account.as_ptr() as _);

let method2 = b"max_receipt_size_promise_return_method2";
let promise_a = promise_create(
current_account.len() as u64,
current_account.as_ptr() as u64,
method2.len() as u64,
method2.as_ptr() as u64,
args.len() as u64, // Forward the args
args.as_ptr() as u64,
0,
200 * TGAS,
);

let empty_args: &[u8] = &[];
let test_completed_method = b"mark_test_completed";
let _promise_b = promise_then(
promise_a,
current_account.len() as u64,
current_account.as_ptr() as u64,
test_completed_method.len() as u64,
test_completed_method.as_ptr() as u64,
empty_args.len() as u64,
empty_args.as_ptr() as u64,
0,
20 * TGAS,
);
}

/// Do a promise_return with a large receipt.
/// The receipt has a single FunctionCall action with large args.
/// Creates DAG:
/// C[self.noop(large_args)] -then-> B[self.mark_test_completed()]
#[no_mangle]
pub unsafe fn max_receipt_size_promise_return_method2() {
input(0);
let args = vec![0u8; register_len(0) as usize];
read_register(0, args.as_ptr() as u64);
let input_args_json: serde_json::Value = serde_json::from_slice(&args).unwrap();
let args_size = input_args_json["args_size"].as_u64().unwrap();

current_account_id(0);
let current_account = vec![0u8; register_len(0) as usize];
read_register(0, current_account.as_ptr() as _);

let large_args = vec![0u8; args_size as usize];
let noop_method = b"noop";
let promise_c = promise_create(
current_account.len() as u64,
current_account.as_ptr() as u64,
noop_method.len() as u64,
noop_method.as_ptr() as u64,
large_args.len() as u64,
large_args.as_ptr() as u64,
0,
20 * TGAS,
);

promise_return(promise_c);
}

/// Mark a test as completed
#[no_mangle]
pub unsafe fn mark_test_completed() {
let key = b"test_completed";
let value = b"true";
storage_write(
key.len() as u64,
key.as_ptr() as u64,
value.len() as u64,
value.as_ptr() as u64,
0,
);
}

// Assert that the test has been marked as completed.
// (Make sure that the method mark_test_completed was executed)
#[no_mangle]
pub unsafe fn assert_test_completed() {
let key = b"test_completed";
let read_res = storage_read(key.len() as u64, key.as_ptr() as u64, 0);
if read_res != 1 {
let panic_msg = b"assert_test_completed failed - can't read test_completed marker";
panic_utf8(panic_msg.len() as u64, panic_msg.as_ptr() as u64);
}

let value = vec![0u8; register_len(0) as usize];
read_register(0, value.as_ptr() as u64);
if value != b"true" {
let panic_msg = b"assert_test_completed failed - test_completed value is not true";
panic_utf8(panic_msg.len() as u64, panic_msg.as_ptr() as u64);
}
}