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 9 commits into
base: master
Choose a base branch
from
238 changes: 237 additions & 1 deletion integration-tests/src/test_loop/tests/max_receipt_size.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
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,
ActionError, ActionErrorKind, FunctionCallError, 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 @@ -122,3 +126,235 @@ 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?

// 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();
let rpc_id = "account4".parse().unwrap();

// 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, &rpc_id, 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, &rpc_id, 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, &rpc_id, assert_test_completed, &env.datas, Duration::seconds(5));

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

/// Return a value that is as large as max_receipt_size. The value will be wrapped in a data receipt
/// and the data receipt will be bigger than max_receipt_size. The receipt should be rejected, but
/// currently isn't because of a bug (See https://github.com/near/nearcore/issues/12606)
/// Creates the following promise DAG:
/// A[self.return_large_value()] -then-> B[self.mark_test_completed()]
#[test]
fn test_max_receipt_size_value_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();
let rpc_id = "account4".parse().unwrap();

// 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, &rpc_id, deploy_contract_tx, &env.datas, Duration::seconds(5));

let max_receipt_size = 4_194_304;

// Call the contract
let large_receipt_tx = SignedTransaction::call(
102,
account.clone(),
account.clone(),
&account_signer,
0,
"max_receipt_size_value_return_method".into(),
format!("{{\"value_size\": {}}}", max_receipt_size).into(),
300 * TGAS,
get_shared_block_hash(&env.datas, &env.test_loop),
);
run_tx(&mut env.test_loop, &rpc_id, 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, &rpc_id, assert_test_completed, &env.datas, Duration::seconds(5));

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

/// Yielding produces a new action receipt, resuming produces a new data receipt.
/// Make sure that the size of receipts produced by yield/resume is limited to below `max_receipt_size`.
#[test]
fn test_max_receipt_size_yield_resume() {
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();
let rpc_id = "account4".parse().unwrap();

// 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, &rpc_id, deploy_contract_tx, &env.datas, Duration::seconds(50));

let max_receipt_size = 4_194_304;

// Perform a yield which creates a receipt that is larger than the max_receipt_size.
// It should be rejected because of the receipt size limit.
let yield_receipt_tx = SignedTransaction::call(
102,
account.clone(),
account.clone(),
&account_signer,
0,
"yield_with_large_args".into(),
format!("{{\"args_size\": {}}}", max_receipt_size).into(),
300 * TGAS,
get_shared_block_hash(&env.datas, &env.test_loop),
);
let yield_receipt_res = execute_tx(
&mut env.test_loop,
&rpc_id,
yield_receipt_tx,
&env.datas,
Duration::seconds(10),
)
.unwrap();

let expected_yield_status =
FinalExecutionStatus::Failure(TxExecutionError::ActionError(ActionError {
index: Some(0),
kind: ActionErrorKind::NewReceiptValidationError(
ReceiptValidationError::ReceiptSizeExceeded {
size: 4194503,
limit: max_receipt_size,
},
),
}));
assert_eq!(yield_receipt_res.status, expected_yield_status);

// Perform a resume which would create a large data receipt.
// It fails because the max payload size is 1024.
// Definitely not exceeding max_receipt_size.
let resume_receipt_tx = SignedTransaction::call(
103,
account.clone(),
account,
&account_signer,
0,
"resume_with_large_payload".into(),
format!("{{\"payload_size\": {}}}", 2000).into(),
300 * TGAS,
get_shared_block_hash(&env.datas, &env.test_loop),
);
let resume_receipt_res = execute_tx(
&mut env.test_loop,
&rpc_id,
resume_receipt_tx,
&env.datas,
Duration::seconds(5),
)
.unwrap();

let expected_resume_status =
FinalExecutionStatus::Failure(TxExecutionError::ActionError(ActionError {
index: Some(0),
kind: ActionErrorKind::FunctionCallError(FunctionCallError::ExecutionError(
"Yield resume payload is 2000 bytes which exceeds the 1024 byte limit".to_string(),
)),
}));
assert_eq!(resume_receipt_res.status, expected_resume_status);

env.shutdown_and_drain_remaining_events(Duration::seconds(20));
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ fn setup_account(
/// it can be called successfully.
fn setup_contract(env: &mut TestEnv, nonce: &mut u64) {
let block = env.clients[0].chain.get_head_block().unwrap();
let contract = near_test_contracts::rs_contract();
let contract = near_test_contracts::congestion_control_test_contract();

let signer_id: AccountId = ACCOUNT_PARENT_ID.parse().unwrap();
let signer = InMemorySigner::test_signer(&signer_id);
Expand Down Expand Up @@ -787,8 +787,8 @@ fn measure_tx_limit(
let congestion_level = congestion_info.localized_congestion_level(&config);
// congestion should be non-trivial and below the upper limit
assert!(
incoming_congestion > upper_limit_congestion / 4.0,
"{incoming_congestion} > {upper_limit_congestion} / 4 failed, {congestion_info:?}"
incoming_congestion > upper_limit_congestion / 2.0,
"{incoming_congestion} > {upper_limit_congestion} / 2 failed, {congestion_info:?}"
);
assert!(
congestion_level < upper_limit_congestion,
Expand Down
5 changes: 5 additions & 0 deletions runtime/near-test-contracts/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ fn try_main() -> Result<(), Error> {
&["--features", &test_contract_features_string],
"test_contract_rs",
)?;
build_contract(
"./congestion-control-test-contract",
&["--features", &test_contract_features_string],
"congestion_control_test_contract",
)?;

test_contract_features.push("nightly");
let test_contract_features_string = test_contract_features.join(",");
Expand Down

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

Loading
Loading