Skip to content

Commit

Permalink
fix(runtime) - Mitigate the receipt size limit bug (#12633)
Browse files Browse the repository at this point in the history
There is a bug (#12606) which
allows to create receipts which are slightly larger than
`max_receipt_size`.

Let's mitigate the issue by making the runtime able to handle receipts
that are above `max_receipt_size`.
First the size limit check is relaxed to check only new receipts. Then
the receipt forwarding logic is modified to pretend that all receipts
have `congestion_size` below `max_receipt_size`.
This mostly matches the code that was added in `2.3.1` and `2.4.0`, but
this PR contains one more change that is needed for bandwidth scheduler.
Before bandwidth scheduler we were able to forward receipts that are
slightly above the `max_receipt_size` because the big outgoing limit was
4.5 MiB and `try_forward` pretended that all receipts are at most 4 MiB.
With bandwidth scheduler this changes, bandwidth scheduler expects all
receipts to be at most `max_receipt_size`, otherwise there's no
guarantee that the receipt will ever be forwarded. To deal with this
there's the change which pretends that all receipts are at most
`max_receipt_size` when generating bandwidth requests.
This is just a mitigation, a proper fix will be done in the future.

One more advantage of this patch is that it should allow us to reduce
`max_receipt_size` without crazy protocol upgrade logic. We could reduce
it to e.g 2MB and the scheduler would still be able to handle 4MB
receipts created in the previous protocol version.

I added some of tests which try to create a receipt above max size and
check that everything still works fine.

As usual the `test_transaction_limit_for_local_congestion` test broke
when I added more methods to the `test-contract-rs` contract. For some
reason this test is very sensitive to contract size and the congestion
level rapidly drops when size of the contract increases. To deal with
this I added a separate small contract that is used exclusively by these
congestion control tests, that should fix the issue once and for all.

The change is divided into commits for easier review.
  • Loading branch information
jancionear authored Jan 8, 2025
1 parent cd21498 commit 83bf747
Show file tree
Hide file tree
Showing 11 changed files with 945 additions and 13 deletions.
323 changes: 322 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,15 @@
use assert_matches::assert_matches;
use near_async::time::Duration;
use near_chain::{ChainStoreAccess, ReceiptFilter};
use near_o11y::testonly::init_test_logger;
use near_primitives::action::{Action, FunctionCallAction};
use near_primitives::block::MaybeNew;
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 +128,318 @@ 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
// 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.data),
);
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.data),
);
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.data),
);
run_tx(&mut env.test_loop, &rpc_id, assert_test_completed, &env.datas, Duration::seconds(5));

assert_oversized_receipt_occurred(&env);

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

assert_oversized_receipt_occurred(&env);

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.data),
);
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.data),
);
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.data),
);
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));
}

/// Assert that there was an incoming receipt with size above max_receipt_size
fn assert_oversized_receipt_occurred(env: &TestLoopEnv) {
let client_handle = env.datas[0].client_sender.actor_handle();
let client = &env.test_loop.data.get(&client_handle).client;
let chain = &client.chain;
let epoch_manager = &*client.epoch_manager;

let tip = chain.head().unwrap();
let mut block = chain.get_block(&tip.last_block_hash).unwrap();
let mut prev_block = chain.get_block(&block.header().prev_hash()).unwrap();

let epoch_id = epoch_manager.get_epoch_id(block.hash()).unwrap();
let protocol_version = epoch_manager.get_epoch_protocol_version(&epoch_id).unwrap();
let runtime_config = client.runtime_adapter.get_runtime_config(protocol_version).unwrap();

// Go over all blocks in the range
loop {
if block.header().is_genesis() {
panic!("Didn't find receipt with size above max_receipt_size!");
}

let cur_shard_layout = client
.epoch_manager
.get_shard_layout(&epoch_manager.get_epoch_id(&block.hash()).unwrap())
.unwrap();

// Go over all new chunks in a block
for chunk_header in block.chunks().iter() {
let MaybeNew::New(new_chunk) = chunk_header else {
continue;
};
let shard_id = new_chunk.shard_id();
let prev_shard_index = epoch_manager
.get_prev_shard_id_from_prev_hash(block.header().prev_hash(), shard_id)
.unwrap()
.2;
let prev_height_included =
prev_block.chunks().get(prev_shard_index).unwrap().height_included();

// Fetch incoming receipts for this chunk
let incoming_receipts_proofs = chain
.chain_store
.get_incoming_receipts_for_shard(
epoch_manager,
shard_id,
&cur_shard_layout,
*block.hash(),
prev_height_included,
ReceiptFilter::TargetShard,
)
.unwrap();

// Look for receipt with size above max_receipt_size
for response in incoming_receipts_proofs {
for proof in response.1.iter() {
for receipt in &proof.0 {
let receipt_size: u64 =
borsh::object_length(receipt).unwrap().try_into().unwrap();
let max_receipt_size =
runtime_config.wasm_config.limit_config.max_receipt_size;
if receipt_size > max_receipt_size {
// Success! found receipt above max size
tracing::info!(
"Found receipt above max size! Receipt size: {}, max size: {}",
receipt_size,
max_receipt_size
);
return;
}
}
}
}
}

block = prev_block;
prev_block = chain.get_block(block.header().prev_hash()).unwrap();
}
}
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
Loading

0 comments on commit 83bf747

Please sign in to comment.