Skip to content

Commit

Permalink
fix(congestion) - Correctly handle receipt gas and size calculation i…
Browse files Browse the repository at this point in the history
…n protocol upgrades (#12031)

For now just a request for comments. There are still a few missing
pieces like adding tests and gating this feature by a protocol feature.
Please let me know if this approach makes sense on a high level.

This PR addresses the issue described in #11923. Please have a look at
that first for context.

This is a hacky implementation of option 1:
"Store the receipts together with the gas and size when added and use
that when removing the receipt from buffer."

The basic idea is that instead of storing `Receipt` directly in state,
it can be wrapped in a `StateStoredReceipt` together with the metadata
that is needed to correctly handle protocol upgrades.

In order to migrate from the old way of storing receipts I'm using a
rather hacky solution. I implemented a custom serialization and
deserialization for the `StateStoredReceipt`. Using that customization
it's possible to discriminate between serialized Receipt and serialized
StateStoredReceipt. I added a helper struct
`ReceiptOrStateStoredReceipt` and I made it so that both Receipt and
StateStoredReceipt can be deserialized directly to it.

How to differentiate between `Receipt` and `StateStoredReceipt` ?
* Receipt::V0 has first two bytes in the form of [x, 0] - the second
byte is zero.
* Receipt::V1 has first two bytes in the form of [1, x] - where x != 0 -
the first byte is one and second is non-zero.
* Receipt::Vn (future) has first two bytes in the form of [n, x] - where
x != 0
* StateStoredReceipt has first two bytes in the for of [T, T] where T is
a magic tag == u8::MAX.

The `StateStoredReceipt` can be told apart from Receipt::V0 by the
second byte.
The `StateStoredReceipt` can be told apart from Receipt::Vn by the first
byte.
Receipt::V0 and Receipt::V1 can be told apart by the second byte as
described in

https://github.com/near/nearcore/blob/0e5e7c7234952d1db8cbbafc984b7c8e6a1ac0ba/core/primitives/src/receipt.rs#L105-L118

How will the migration from `Receipt` to `StateStoredReceipt` happen? 
* In the old protocol version receipts will be stored as `Receipt`
* In the new protocol version receipts will be stored as
`StateStoredReceipt`
* In both version receipts will be read as
`ReceiptOrStateStoredReceipt`. This covers the fun case where receipts
are stored in the old version and read in the new version.
  • Loading branch information
wacban authored Sep 23, 2024
1 parent e516989 commit 7ad85a6
Show file tree
Hide file tree
Showing 22 changed files with 648 additions and 86 deletions.
9 changes: 6 additions & 3 deletions chain/client/src/test_utils/test_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,9 +808,12 @@ impl TestEnv {
pub fn print_block_summary(&self, height: u64) {
let client = &self.clients[0];
let block = client.chain.get_block_by_height(height);
let Ok(block) = block else {
tracing::info!(target: "test", "Block {}: missing", height);
return;
let block = match block {
Ok(block) => block,
Err(err) => {
tracing::info!(target: "test", ?err, "Block {}: missing", height);
return;
}
};
let prev_hash = block.header().prev_hash();
let epoch_id = client.epoch_manager.get_epoch_id_from_prev_block(prev_hash).unwrap();
Expand Down
3 changes: 3 additions & 0 deletions core/parameters/res/runtime_configs/72.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
main_storage_proof_size_soft_limit: {old: 3_000_000, new: 4_000_000}

use_state_stored_receipt: { old: false, new: true }

# See https://github.com/near/nearcore/pull/12044 for why the values are set to these values.
# In addition, `gas` is set to 1 for the large read variants, because we need that in actual code.
# For this to be transparent for smart contracts, the `read_base` and `read_value_byte` values were
Expand Down
1 change: 1 addition & 0 deletions core/parameters/res/runtime_configs/parameters.snap
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,4 @@ allowed_shard_outgoing_gas 1_000_000_000_000_000
max_tx_gas 500_000_000_000_000
min_tx_gas 20_000_000_000_000
reject_tx_congestion_threshold 50 / 100
use_state_stored_receipt true
2 changes: 2 additions & 0 deletions core/parameters/res/runtime_configs/parameters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,5 @@ reject_tx_congestion_threshold: {
numerator: 1,
denominator: 1,
}

use_state_stored_receipt: false
2 changes: 2 additions & 0 deletions core/parameters/res/runtime_configs/parameters_testnet.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -259,3 +259,5 @@ reject_tx_congestion_threshold: {
numerator: 1,
denominator: 1,
}

use_state_stored_receipt: false
5 changes: 5 additions & 0 deletions core/parameters/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ pub struct RuntimeConfig {
pub congestion_control_config: CongestionControlConfig,
/// Configuration specific to ChunkStateWitness.
pub witness_config: WitnessConfig,

/// Whether receipts should be stored as [StateStoredReceipt].
pub use_state_stored_receipt: bool,
}

impl RuntimeConfig {
Expand Down Expand Up @@ -59,6 +62,7 @@ impl RuntimeConfig {
account_creation_config: AccountCreationConfig::default(),
congestion_control_config: runtime_config.congestion_control_config,
witness_config: runtime_config.witness_config,
use_state_stored_receipt: runtime_config.use_state_stored_receipt,
}
}

Expand All @@ -75,6 +79,7 @@ impl RuntimeConfig {
account_creation_config: AccountCreationConfig::default(),
congestion_control_config: runtime_config.congestion_control_config,
witness_config: runtime_config.witness_config,
use_state_stored_receipt: runtime_config.use_state_stored_receipt,
}
}

Expand Down
8 changes: 7 additions & 1 deletion core/parameters/src/config_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ static CONFIG_DIFFS: &[(ProtocolVersion, &str)] = &[
(69, include_config!("69.yaml")),
// Introduce ETH-implicit accounts.
(70, include_config!("70.yaml")),
// Increase main_storage_proof_size_soft_limit
// Increase main_storage_proof_size_soft_limit and introduces StateStoredReceipt
(72, include_config!("72.yaml")),
(129, include_config!("129.yaml")),
];
Expand Down Expand Up @@ -119,6 +119,7 @@ impl RuntimeConfigStore {
account_creation_config: runtime_config.account_creation_config.clone(),
congestion_control_config: runtime_config.congestion_control_config,
witness_config: runtime_config.witness_config,
use_state_stored_receipt: runtime_config.use_state_stored_receipt,
}),
);
store.insert(0, Arc::new(runtime_config.clone()));
Expand Down Expand Up @@ -164,6 +165,11 @@ impl RuntimeConfigStore {
Self { store: BTreeMap::from_iter([(0, Arc::new(runtime_config))].iter().cloned()) }
}

/// Constructs store with custom configs. This should only be used for testing.
pub fn new_custom(store: BTreeMap<ProtocolVersion, Arc<RuntimeConfig>>) -> Self {
Self { store }
}

/// Constructs test store.
pub fn test() -> Self {
Self::with_one_config(RuntimeConfig::test())
Expand Down
3 changes: 3 additions & 0 deletions core/parameters/src/parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ pub enum Parameter {
MaxTxGas,
MinTxGas,
RejectTxCongestionThreshold,

// Use the StateStoredReceipt structure when storing receipts in State.
UseStateStoredReceipt,
}

#[derive(
Expand Down
1 change: 1 addition & 0 deletions core/parameters/src/parameter_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ impl TryFrom<&ParameterTable> for RuntimeConfig {
new_transactions_validation_state_size_soft_limit: params
.get(Parameter::NewTransactionsValidationStateSizeSoftLimit)?,
},
use_state_stored_receipt: params.get(Parameter::UseStateStoredReceipt)?,
})
}
}
Expand Down
5 changes: 4 additions & 1 deletion core/primitives-core/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ pub enum ProtocolFeature {
// in order to calculate the rewards and kickouts for the chunk validators.
// This feature introduces BlockHeaderV5.
ChunkEndorsementsInBlockHeader,
/// Store receipts in State in the StateStoredReceipt format.
StateStoredReceipt,
}

impl ProtocolFeature {
Expand Down Expand Up @@ -226,7 +228,8 @@ impl ProtocolFeature {
ProtocolFeature::FixMinStakeRatio => 71,
ProtocolFeature::IncreaseStorageProofSizeSoftLimit
| ProtocolFeature::ChunkEndorsementV2
| ProtocolFeature::ChunkEndorsementsInBlockHeader => 72,
| ProtocolFeature::ChunkEndorsementsInBlockHeader
| ProtocolFeature::StateStoredReceipt => 72,

// This protocol version is reserved for use in resharding tests. An extra resharding
// is simulated on top of the latest shard layout in production. Note that later
Expand Down
Loading

0 comments on commit 7ad85a6

Please sign in to comment.