Skip to content

Commit

Permalink
refactor: Fix delayed receipts trie key column identifier (#10370)
Browse files Browse the repository at this point in the history
Currently both `TrieKey::DelayedReceipt` and
`TrieKey::DelayedReceiptIndices` are serialized with
`col::DELAYED_RECEIPT_INDICES`, probably due to a typo in the original
implementation. It was accidentally discovered when debugging an
unrelated issue using mainnet State.
Changing this is quite involved and requires data migration for State
and FlatState rocksdb columns. This PR makes existing behaviour more
explicit and adds a bit of documntation.
See also [the corresponding zulip
thread](https://near.zulipchat.com/#narrow/stream/295302-general/topic/Delayed.20receipts.20serialized.20with.20the.20wrong.20column.20id.3F).
  • Loading branch information
pugachAG authored Jan 2, 2024
1 parent ac4db89 commit 7365af4
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 25 deletions.
10 changes: 7 additions & 3 deletions core/primitives/src/state_record.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use crate::account::{AccessKey, Account};
use crate::hash::{hash, CryptoHash};
use crate::receipt::{Receipt, ReceivedData};
use crate::trie_key::col;
use crate::trie_key::trie_key_parsers::{
parse_account_id_from_access_key_key, parse_account_id_from_account_key,
parse_account_id_from_contract_code_key, parse_account_id_from_contract_data_key,
parse_account_id_from_received_data_key, parse_data_id_from_received_data_key,
parse_data_key_from_contract_data_key, parse_public_key_from_access_key_key,
};
use crate::trie_key::{col, TrieKey};
use crate::types::{AccountId, StoreKey, StoreValue};
use borsh::BorshDeserialize;
use near_crypto::PublicKey;
Expand Down Expand Up @@ -94,11 +94,15 @@ impl StateRecord {
let receipt = Receipt::try_from_slice(&value)?;
Some(StateRecord::PostponedReceipt(Box::new(receipt)))
}
col::DELAYED_RECEIPT => {
col::DELAYED_RECEIPT_OR_INDICES
if key.len() == TrieKey::DelayedReceiptIndices.len() =>
{
None
}
col::DELAYED_RECEIPT_OR_INDICES => {
let receipt = Receipt::try_from_slice(&value)?;
Some(StateRecord::DelayedReceipt(Box::new(receipt)))
}
col::DELAYED_RECEIPT_INDICES => None,
_ => {
println!("key[0]: {} is unreachable", key[0]);
None
Expand Down
22 changes: 13 additions & 9 deletions core/primitives/src/trie_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ pub mod col {
pub const PENDING_DATA_COUNT: u8 = 5;
/// This column id is used when storing the postponed receipts (`primitives::receipt::Receipt`).
pub const POSTPONED_RECEIPT: u8 = 6;
/// This column id is used when storing the indices of the delayed receipts queue.
/// NOTE: It is a singleton per shard.
pub const DELAYED_RECEIPT_INDICES: u8 = 7;
/// This column id is used when storing delayed receipts, because the shard is overwhelmed.
pub const DELAYED_RECEIPT: u8 = 8;
/// This column id is used when storing:
/// * the indices of the delayed receipts queue (a singleton per shard)
/// * the delayed receipts themselves
/// The identifier is shared between two different key types for for historical reasons. It
/// is valid because the length of `TrieKey::DelayedReceipt` is always greater than
/// `TrieKey::DelayedReceiptIndices` when serialized to bytes.
pub const DELAYED_RECEIPT_OR_INDICES: u8 = 7;
/// This column id is used when storing Key-Value data from a contract on an `account_id`.
pub const CONTRACT_DATA: u8 = 9;
/// All columns
Expand Down Expand Up @@ -138,8 +140,10 @@ impl TrieKey {
+ ACCOUNT_DATA_SEPARATOR.len()
+ receipt_id.as_ref().len()
}
TrieKey::DelayedReceiptIndices => col::DELAYED_RECEIPT_INDICES.len(),
TrieKey::DelayedReceipt { .. } => col::DELAYED_RECEIPT.len() + size_of::<u64>(),
TrieKey::DelayedReceiptIndices => col::DELAYED_RECEIPT_OR_INDICES.len(),
TrieKey::DelayedReceipt { .. } => {
col::DELAYED_RECEIPT_OR_INDICES.len() + size_of::<u64>()
}
TrieKey::ContractData { account_id, key } => {
col::CONTRACT_DATA.len()
+ account_id.len()
Expand Down Expand Up @@ -193,10 +197,10 @@ impl TrieKey {
buf.extend(receipt_id.as_ref());
}
TrieKey::DelayedReceiptIndices => {
buf.push(col::DELAYED_RECEIPT_INDICES);
buf.push(col::DELAYED_RECEIPT_OR_INDICES);
}
TrieKey::DelayedReceipt { index } => {
buf.push(col::DELAYED_RECEIPT_INDICES);
buf.push(col::DELAYED_RECEIPT_OR_INDICES);
buf.extend(&index.to_le_bytes());
}
TrieKey::ContractData { account_id, key } => {
Expand Down
3 changes: 1 addition & 2 deletions nearcore/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,7 @@ mod tests {
create_item(&[col::POSTPONED_RECEIPT, 1]),
create_item(&[col::POSTPONED_RECEIPT, 2]),
create_item(&[col::POSTPONED_RECEIPT, 3]),
create_item(&[col::DELAYED_RECEIPT_INDICES, 1]),
create_item(&[col::DELAYED_RECEIPT, 1]),
create_item(&[col::DELAYED_RECEIPT_OR_INDICES, 1]),
create_item(&[col::CONTRACT_DATA, 1]),
];
let count = get_postponed_receipt_count_for_trie(create_trie(&items)).unwrap();
Expand Down
11 changes: 4 additions & 7 deletions tools/state-viewer/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,7 @@ pub enum RecordType {
PostponedReceiptId = col::POSTPONED_RECEIPT_ID,
PendingDataCount = col::PENDING_DATA_COUNT,
PostponedReceipt = col::POSTPONED_RECEIPT,
DelayedReceiptIndices = col::DELAYED_RECEIPT_INDICES,
DelayedReceipt = col::DELAYED_RECEIPT,
DelayedReceiptOrIndices = col::DELAYED_RECEIPT_OR_INDICES,
ContractData = col::CONTRACT_DATA,
}

Expand All @@ -692,8 +691,7 @@ impl clap::ValueEnum for RecordType {
Self::PostponedReceiptId,
Self::PendingDataCount,
Self::PostponedReceipt,
Self::DelayedReceiptIndices,
Self::DelayedReceipt,
Self::DelayedReceiptOrIndices,
Self::ContractData,
]
}
Expand All @@ -709,10 +707,9 @@ impl clap::ValueEnum for RecordType {
}
Self::PendingDataCount => Some(clap::builder::PossibleValue::new("pending-data-count")),
Self::PostponedReceipt => Some(clap::builder::PossibleValue::new("postponed-receipt")),
Self::DelayedReceiptIndices => {
Some(clap::builder::PossibleValue::new("delayed-receipt-indices"))
Self::DelayedReceiptOrIndices => {
Some(clap::builder::PossibleValue::new("delayed-receipt-or-indices"))
}
Self::DelayedReceipt => Some(clap::builder::PossibleValue::new("delayed-receipt")),
Self::ContractData => Some(clap::builder::PossibleValue::new("contract-data")),
}
}
Expand Down
6 changes: 2 additions & 4 deletions tools/state-viewer/src/trie_iteration_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ impl ColumnCountMap {
match col {
col::ACCOUNT => "ACCOUNT",
col::CONTRACT_CODE => "CONTRACT_CODE",
col::DELAYED_RECEIPT => "DELAYED_RECEIPT",
col::DELAYED_RECEIPT_INDICES => "DELAYED_RECEIPT_INDICES",
col::DELAYED_RECEIPT_OR_INDICES => "DELAYED_RECEIPT_OR_INDICES",
col::ACCESS_KEY => "ACCESS_KEY",
col::CONTRACT_DATA => "CONTRACT_DATA",
col::RECEIVED_DATA => "RECEIVED_DATA",
Expand Down Expand Up @@ -234,9 +233,8 @@ impl TrieIterationBenchmarkCmd {
// key for contract code only contains account id, nothing to prune
col::CONTRACT_CODE => false,
// key for delayed receipt only contains account id, nothing to prune
col::DELAYED_RECEIPT => false,
// key for delayed receipt indices is a shard singleton, nothing to prune
col::DELAYED_RECEIPT_INDICES => false,
col::DELAYED_RECEIPT_OR_INDICES => false,

// Most columns use the ACCOUNT_DATA_SEPARATOR to indicate the end
// of the accound id in the trie key. For those columns the
Expand Down

0 comments on commit 7365af4

Please sign in to comment.