Skip to content

Commit

Permalink
fix: resolve pr concerns
Browse files Browse the repository at this point in the history
  • Loading branch information
KolbyML committed Sep 16, 2024
1 parent a2147d3 commit d091da2
Show file tree
Hide file tree
Showing 33 changed files with 255 additions and 241 deletions.
63 changes: 44 additions & 19 deletions ethportal-api/src/types/content_key/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ pub const HISTORY_BLOCK_HEADER_BY_NUMBER_KEY_PREFIX: u8 = 0x03;
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum HistoryContentKey {
/// A block header with accumulator proof.
BlockHeaderByHashWithProof(BlockHeaderByHashKey),
BlockHeaderByHash(BlockHeaderByHashKey),
/// A block header with accumulator proof.
BlockHeaderByNumberWithProof(BlockHeaderByNumberKey),
BlockHeaderByNumber(BlockHeaderByNumberKey),
/// A block body.
BlockBody(BlockBodyKey),
/// The transaction receipts for a block.
Expand Down Expand Up @@ -142,7 +142,7 @@ impl TryFrom<RawContentKey> for HistoryContentKey {
};
match selector {
HISTORY_BLOCK_HEADER_BY_HASH_KEY_PREFIX => BlockHeaderByHashKey::from_ssz_bytes(key)
.map(Self::BlockHeaderByHashWithProof)
.map(Self::BlockHeaderByHash)
.map_err(|e| ContentKeyError::from_decode_error(e, value)),
HISTORY_BLOCK_BODY_KEY_PREFIX => BlockBodyKey::from_ssz_bytes(key)
.map(Self::BlockBody)
Expand All @@ -152,7 +152,7 @@ impl TryFrom<RawContentKey> for HistoryContentKey {
.map_err(|e| ContentKeyError::from_decode_error(e, value)),
HISTORY_BLOCK_HEADER_BY_NUMBER_KEY_PREFIX => {
BlockHeaderByNumberKey::from_ssz_bytes(key)
.map(Self::BlockHeaderByNumberWithProof)
.map(Self::BlockHeaderByNumber)
.map_err(|e| ContentKeyError::from_decode_error(e, value))
}
_ => Err(ContentKeyError::from_decode_error(
Expand All @@ -166,8 +166,8 @@ impl TryFrom<RawContentKey> for HistoryContentKey {
impl fmt::Display for HistoryContentKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let s = match self {
Self::BlockHeaderByHashWithProof(header) => format!(
"BlockHeaderByHashWithProof {{ block_hash: {} }}",
Self::BlockHeaderByHash(header) => format!(
"BlockHeaderByHash {{ block_hash: {} }}",
hex_encode_compact(header.block_hash)
),
Self::BlockBody(body) => format!(
Expand All @@ -180,9 +180,9 @@ impl fmt::Display for HistoryContentKey {
hex_encode_compact(receipts.block_hash)
)
}
Self::BlockHeaderByNumberWithProof(header) => {
Self::BlockHeaderByNumber(header) => {
format!(
"BlockHeaderByNumberWithProof {{ block_number: {} }}",
"BlockHeaderByNumber {{ block_number: {} }}",
header.block_number
)
}
Expand All @@ -203,7 +203,7 @@ impl OverlayContentKey for HistoryContentKey {
let mut bytes: Vec<u8> = Vec::new();

match self {
HistoryContentKey::BlockHeaderByHashWithProof(k) => {
HistoryContentKey::BlockHeaderByHash(k) => {
bytes.push(HISTORY_BLOCK_HEADER_BY_HASH_KEY_PREFIX);
bytes.extend_from_slice(&k.block_hash);
}
Expand All @@ -215,7 +215,7 @@ impl OverlayContentKey for HistoryContentKey {
bytes.push(HISTORY_BLOCK_RECEIPTS_KEY_PREFIX);
bytes.extend_from_slice(&k.block_hash);
}
HistoryContentKey::BlockHeaderByNumberWithProof(k) => {
HistoryContentKey::BlockHeaderByNumber(k) => {
bytes.push(HISTORY_BLOCK_HEADER_BY_NUMBER_KEY_PREFIX);
bytes.extend_from_slice(&k.block_number.as_ssz_bytes());
}
Expand Down Expand Up @@ -252,13 +252,13 @@ mod test {
block_hash: BLOCK_HASH,
};

let key = HistoryContentKey::BlockHeaderByHashWithProof(header);
let key = HistoryContentKey::BlockHeaderByHash(header);

assert_eq!(key.to_bytes(), expected_content_key);
assert_eq!(key.content_id(), expected_content_id);
assert_eq!(
key.to_string(),
"BlockHeaderByHashWithProof { block_hash: 0xd1c3..621d }"
"BlockHeaderByHash { block_hash: 0xd1c3..621d }"
);
assert_eq!(key.to_hex(), KEY_STR);
}
Expand All @@ -278,13 +278,17 @@ mod test {
block_number: BLOCK_NUMBER,
};

let key = HistoryContentKey::BlockHeaderByNumberWithProof(header);
let key = HistoryContentKey::BlockHeaderByNumber(header);

// round trip
let decoded = HistoryContentKey::try_from(key.to_bytes()).unwrap();
assert_eq!(decoded, key);

assert_eq!(key.to_bytes(), expected_content_key);
assert_eq!(key.content_id(), expected_content_id);
assert_eq!(
key.to_string(),
"BlockHeaderByNumberWithProof { block_number: 12345678 }"
"BlockHeaderByNumber { block_number: 12345678 }"
);
assert_eq!(key.to_hex(), KEY_STR);
}
Expand All @@ -306,6 +310,10 @@ mod test {

let key = HistoryContentKey::BlockBody(body);

// round trip
let decoded = HistoryContentKey::try_from(key.to_bytes()).unwrap();
assert_eq!(decoded, key);

assert_eq!(key.to_bytes(), expected_content_key);
assert_eq!(key.content_id(), expected_content_id);
assert_eq!(key.to_string(), "BlockBody { block_hash: 0xd1c3..621d }");
Expand Down Expand Up @@ -339,13 +347,30 @@ mod test {
}

#[test]
fn ser_de_block_header() {
fn ser_de_block_header_by_hash() {
let content_key_json =
"\"0x00d1c390624d3bd4e409a61a858e5dcc5517729a9170d014a6c96530d64dd8621d\"";
let expected_content_key =
HistoryContentKey::BlockHeaderByHashWithProof(BlockHeaderByHashKey {
block_hash: BLOCK_HASH,
});
let expected_content_key = HistoryContentKey::BlockHeaderByHash(BlockHeaderByHashKey {
block_hash: BLOCK_HASH,
});

let content_key: HistoryContentKey = serde_json::from_str(content_key_json).unwrap();

assert_eq!(content_key, expected_content_key);
assert_eq!(
serde_json::to_string(&content_key).unwrap(),
content_key_json
);
}

#[test]
fn ser_de_block_header_by_number() {
// let content_key_json = "12345678";
let content_key_json = "\"0x034e61bc0000000000\"";

let expected_content_key = HistoryContentKey::BlockHeaderByNumber(BlockHeaderByNumberKey {
block_number: 12345678,
});

let content_key: HistoryContentKey = serde_json::from_str(content_key_json).unwrap();

Expand Down
3 changes: 1 addition & 2 deletions ethportal-api/src/types/content_value/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ impl ContentValue for HistoryContentValue {

fn decode(key: &Self::TContentKey, buf: &[u8]) -> Result<Self, ContentValueError> {
match key {
HistoryContentKey::BlockHeaderByHashWithProof(_)
| HistoryContentKey::BlockHeaderByNumberWithProof(_) => {
HistoryContentKey::BlockHeaderByHash(_) | HistoryContentKey::BlockHeaderByNumber(_) => {
if let Ok(value) = HeaderWithProof::from_ssz_bytes(buf) {
return Ok(Self::BlockHeaderWithProof(value));
}
Expand Down
6 changes: 3 additions & 3 deletions ethportal-peertest/src/scenarios/basic.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{utils::fixture_header_with_proof, Peertest, PeertestNode};
use crate::{utils::fixture_header_by_hash, Peertest, PeertestNode};
use alloy_primitives::{B256, U256};
use ethportal_api::{
types::{
Expand Down Expand Up @@ -177,7 +177,7 @@ pub async fn test_find_nodes_zero_distance(

pub async fn test_history_store(target: &Client) {
info!("Testing portal_historyStore");
let (content_key, content_value) = fixture_header_with_proof();
let (content_key, content_value) = fixture_header_by_hash();
let result = HistoryNetworkApiClient::store(target, content_key, content_value.encode())
.await
.unwrap();
Expand All @@ -186,7 +186,7 @@ pub async fn test_history_store(target: &Client) {

pub async fn test_history_local_content_absent(target: &Client) {
info!("Testing portal_historyLocalContent absent");
let content_key = HistoryContentKey::BlockHeaderByHashWithProof(BlockHeaderByHashKey {
let content_key = HistoryContentKey::BlockHeaderByHash(BlockHeaderByHashKey {
block_hash: B256::random().into(),
});
let error = HistoryNetworkApiClient::local_content(target, content_key)
Expand Down
4 changes: 2 additions & 2 deletions ethportal-peertest/src/scenarios/bridge.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
utils::{fixture_header_with_proof_1000010, wait_for_beacon_content, wait_for_history_content},
utils::{fixture_header_by_hash_1000010, wait_for_beacon_content, wait_for_history_content},
Peertest,
};
use ethportal_api::{
Expand Down Expand Up @@ -38,7 +38,7 @@ pub async fn test_history_bridge(peertest: &Peertest, portal_client: &HttpClient
DEFAULT_GOSSIP_LIMIT,
);
bridge.launch().await;
let (content_key, content_value) = fixture_header_with_proof_1000010();
let (content_key, content_value) = fixture_header_by_hash_1000010();
// Check if the stored content value in bootnode's DB matches the offered
let received_content_value =
wait_for_history_content(&peertest.bootnode.ipc_client, content_key).await;
Expand Down
10 changes: 5 additions & 5 deletions ethportal-peertest/src/scenarios/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use discv5::enr::NodeId;
use jsonrpsee::async_client::Client;
use tracing::info;

use crate::{utils::fixture_header_with_proof, Peertest};
use crate::{utils::fixture_header_by_hash, Peertest};
use ethportal_api::{
types::{portal::ContentInfo, portal_wire::ProtocolId},
utils::bytes::hex_decode,
Expand Down Expand Up @@ -45,7 +45,7 @@ pub async fn test_recursive_find_nodes_random(protocol: ProtocolId, peertest: &P
pub async fn test_find_content_return_enr(target: &Client, peertest: &Peertest) {
info!("Testing find content returns enrs properly");

let (content_key, _) = fixture_header_with_proof();
let (content_key, _) = fixture_header_by_hash();

// check if we can fetch data from routing table
match HistoryNetworkApiClient::get_enr(
Expand Down Expand Up @@ -83,7 +83,7 @@ pub async fn test_find_content_return_enr(target: &Client, peertest: &Peertest)

pub async fn test_trace_recursive_find_content(peertest: &Peertest) {
info!("Testing trace recursive find content");
let (content_key, content_value) = fixture_header_with_proof();
let (content_key, content_value) = fixture_header_by_hash();
let store_result = HistoryNetworkApiClient::store(
&peertest.bootnode.ipc_client,
content_key.clone(),
Expand Down Expand Up @@ -138,7 +138,7 @@ pub async fn test_trace_recursive_find_content(peertest: &Peertest) {
// This test ensures that when content is not found the correct response is returned.
pub async fn test_trace_recursive_find_content_for_absent_content(peertest: &Peertest) {
let client = &peertest.nodes[0].ipc_client;
let (content_key, _) = fixture_header_with_proof();
let (content_key, _) = fixture_header_by_hash();

let error = HistoryNetworkApiClient::trace_recursive_find_content(client, content_key)
.await
Expand All @@ -151,7 +151,7 @@ pub async fn test_trace_recursive_find_content_for_absent_content(peertest: &Pee
}

pub async fn test_trace_recursive_find_content_local_db(peertest: &Peertest) {
let (content_key, content_value) = fixture_header_with_proof();
let (content_key, content_value) = fixture_header_by_hash();

let store_result = HistoryNetworkApiClient::store(
&peertest.bootnode.ipc_client,
Expand Down
Loading

0 comments on commit d091da2

Please sign in to comment.