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

refactor: update jsonrpc endpoint from *RecursiveFindContent to *GetContent #1526

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions book/src/developers/protocols/json_rpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ The specification for these endpoints can be found [here](https://playground.ope
- `portal_historyLocalContent`
- `portal_historyPing`
- `portal_historyOffer`
- `portal_historyRecursiveFindContent`
- `portal_historyGetContent`
- `portal_historyStore`
- `portal_stateFindContent`
- `portal_stateFindNodes`
Expand All @@ -30,7 +30,7 @@ The specification for these endpoints can be found [here](https://playground.ope
The following endpoints are not part of the Portal Network specification and are defined
in subsequent sections:
- [`portal_historyRadius`](#portal_historyradius)
- [`portal_historyTraceRecursiveFindContent`](#portal_historytracerecursivefindcontent)
- [`portal_historyTraceGetContent`](#portal_historytracegetcontent)
- [`portal_paginateLocalContentKeys`](#portal_paginatelocalcontentkeys)
- [`portal_stateRadius`](#portal_stateradius)

Expand All @@ -54,8 +54,8 @@ Returns the current data storage radius being used for the History network.
}
```

## `portal_historyTraceRecursiveFindContent`
Same as `portal_historyRecursiveFindContent`, but will also return a "route" with the content. The "route" contains all of the ENR's contacted during the lookup, and their respective distance to the target content. If the content is available in local storage, the route will contain an empty array.
## `portal_historyTraceGetContent`
Same as `portal_historyGetContent`, but will also return a "route" with the content. The "route" contains all of the ENR's contacted during the lookup, and their respective distance to the target content. If the content is available in local storage, the route will contain an empty array.

### Parameters
- `content_key`: Target content key.
Expand Down
8 changes: 4 additions & 4 deletions book/src/users/use/portal_network_data.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@ Let us request the block body for block 16624561
- Block hash: `0xd27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17`
- Selector for a block body: `0x01` (defined in Portal Network spec under the History sub-protocol).
- Content key: `0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17`
- Request: `portal_historyRecursiveFindContent`, which accepts a content key as a parameter
- Request: `portal_historyGetContent`, which accepts a content key as a parameter

```json
{"jsonrpc":"2.0","method":"portal_historyRecursiveFindContent","params":["0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17"],"id":1}
{"jsonrpc":"2.0","method":"portal_historyGetContent","params":["0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17"],"id":1}
```
## HTTP

```sh
curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","method":"portal_historyRecursiveFindContent","params":["0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17"],"id":1}' http://localhost:8545 | jq
curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","method":"portal_historyGetContent","params":["0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17"],"id":1}' http://localhost:8545 | jq
```

## IPC

```sh
echo '{"jsonrpc":"2.0","method":"portal_historyRecursiveFindContent","params":["0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17"],"id":1}' | nc -U /tmp/trin-jsonrpc.ipc | jq
echo '{"jsonrpc":"2.0","method":"portal_historyGetContent","params":["0x01d27f5e55d88b447788667b3d72cca66b7c944160f68f0a62aaf02aa7e4b2af17"],"id":1}' | nc -U /tmp/trin-jsonrpc.ipc | jq
```
16 changes: 7 additions & 9 deletions ethportal-api/src/beacon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,15 @@ pub trait BeaconNetworkApi {
async fn find_content(&self, enr: Enr, content_key: BeaconContentKey)
-> RpcResult<ContentInfo>;

/// Lookup a target content key in the network
#[method(name = "beaconRecursiveFindContent")]
async fn recursive_find_content(&self, content_key: BeaconContentKey)
-> RpcResult<ContentInfo>;
/// First checks local storage if content is not found lookup a target content key in the
/// network
#[method(name = "beaconGetContent")]
async fn get_content(&self, content_key: BeaconContentKey) -> RpcResult<ContentInfo>;

/// Lookup a target content key in the network. Return tracing info.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think you forgot to update this doc (you did it in other files)

#[method(name = "beaconTraceRecursiveFindContent")]
async fn trace_recursive_find_content(
&self,
content_key: BeaconContentKey,
) -> RpcResult<TraceContentInfo>;
#[method(name = "beaconTraceGetContent")]
async fn trace_get_content(&self, content_key: BeaconContentKey)
-> RpcResult<TraceContentInfo>;

/// Pagination of local content keys
#[method(name = "beaconPaginateLocalContentKeys")]
Expand Down
19 changes: 9 additions & 10 deletions ethportal-api/src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,15 @@ pub trait HistoryNetworkApi {
content_key: HistoryContentKey,
) -> RpcResult<ContentInfo>;

/// Lookup a target content key in the network
#[method(name = "historyRecursiveFindContent")]
async fn recursive_find_content(
&self,
content_key: HistoryContentKey,
) -> RpcResult<ContentInfo>;

/// Lookup a target content key in the network. Return tracing info.
#[method(name = "historyTraceRecursiveFindContent")]
async fn trace_recursive_find_content(
/// First checks local storage if content is not found lookup a target content key in the
/// network
#[method(name = "historyGetContent")]
async fn get_content(&self, content_key: HistoryContentKey) -> RpcResult<ContentInfo>;

/// First checks local storage if content is not found lookup a target content key in the
/// network. Return tracing info.
#[method(name = "historyTraceGetContent")]
async fn trace_get_content(
&self,
content_key: HistoryContentKey,
) -> RpcResult<TraceContentInfo>;
Expand Down
19 changes: 9 additions & 10 deletions ethportal-api/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,15 @@ pub trait StateNetworkApi {
#[method(name = "stateFindContent")]
async fn find_content(&self, enr: Enr, content_key: StateContentKey) -> RpcResult<ContentInfo>;

/// Lookup a target content key in the network
#[method(name = "stateRecursiveFindContent")]
async fn recursive_find_content(&self, content_key: StateContentKey) -> RpcResult<ContentInfo>;

/// Lookup a target content key in the network. Return tracing info.
#[method(name = "stateTraceRecursiveFindContent")]
async fn trace_recursive_find_content(
&self,
content_key: StateContentKey,
) -> RpcResult<TraceContentInfo>;
/// First checks local storage if content is not found lookup a target content key in the
/// network
#[method(name = "stateGetContent")]
async fn get_content(&self, content_key: StateContentKey) -> RpcResult<ContentInfo>;

/// First checks local storage if content is not found lookup a target content key in the
/// network. Return tracing info.
#[method(name = "stateTraceGetContent")]
async fn trace_get_content(&self, content_key: StateContentKey) -> RpcResult<TraceContentInfo>;

/// Pagination of local content keys
#[method(name = "statePaginateLocalContentKeys")]
Expand Down
12 changes: 6 additions & 6 deletions ethportal-api/src/types/jsonrpc/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ pub enum StateEndpoint {
/// params: [enr, content_key]
FindContent(Enr, StateContentKey),
/// params: content_key
RecursiveFindContent(StateContentKey),
GetContent(StateContentKey),
/// params: content_key
TraceRecursiveFindContent(StateContentKey),
TraceGetContent(StateContentKey),
/// params: [content_key, content_value]
Store(StateContentKey, StateContentValue),
/// params: [enr, Vec<(content_key, content_value>)]
Expand Down Expand Up @@ -84,9 +84,9 @@ pub enum HistoryEndpoint {
/// params: [enr]
Ping(Enr),
/// params: content_key
RecursiveFindContent(HistoryContentKey),
GetContent(HistoryContentKey),
/// params: content_key
TraceRecursiveFindContent(HistoryContentKey),
TraceGetContent(HistoryContentKey),
/// params: [content_key, content_value]
Store(HistoryContentKey, HistoryContentValue),
/// params: None
Expand Down Expand Up @@ -136,9 +136,9 @@ pub enum BeaconEndpoint {
/// params: enr
Ping(Enr),
/// params: content_key
RecursiveFindContent(BeaconContentKey),
GetContent(BeaconContentKey),
/// params: content_key
TraceRecursiveFindContent(BeaconContentKey),
TraceGetContent(BeaconContentKey),
/// params: [content_key, content_value]
Store(BeaconContentKey, BeaconContentValue),
/// params: None
Expand Down
12 changes: 2 additions & 10 deletions ethportal-api/src/types/portal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ pub type RawContentValue = Bytes;
pub type DataRadius = U256;
pub type Distance = U256;

/// Part of a TraceRecursiveFindContent response
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct NodeInfo {
pub enr: Enr,
pub distance: Distance,
}

/// Response for Ping endpoint
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -53,7 +45,7 @@ pub struct TraceGossipInfo {
pub transferred: Vec<String>,
}

/// Response for FindContent & RecursiveFindContent endpoints
/// Response for FindContent & GetContent endpoints
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's true that this is the response type, RecursiveFindContent would only return Content variant (this is also according to spec). Should we create separate type for it?

I also believe that ConnectionId variant is never used (not even for FindContent). Should we remove it?

While it's not strictly related to this pr (renaming endpoint), I think that this is something that we should improve. If you feel like it, maybe do it in this PR, otherwise create an issue for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make a follow up PR to resolve this

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
#[allow(clippy::large_enum_variant)]
Expand All @@ -69,7 +61,7 @@ pub enum ContentInfo {
Enrs { enrs: Vec<Enr> },
}

/// Parsed response for TraceRecursiveFindContent endpoint
/// Parsed response for TraceGetContent endpoint
///
/// This struct represents the content info, and is only used
/// when the content is found locally or on the network.
Expand Down
20 changes: 9 additions & 11 deletions ethportal-peertest/src/scenarios/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub async fn test_find_content_return_enr(target: &Client, peertest: &Peertest)
}
}

pub async fn test_trace_recursive_find_content(peertest: &Peertest) {
pub async fn test_trace_get_content(peertest: &Peertest) {
info!("Testing trace recursive find content");
let (content_key, content_value) = fixture_header_by_hash();
let store_result = HistoryNetworkApiClient::store(
Expand All @@ -95,7 +95,7 @@ pub async fn test_trace_recursive_find_content(peertest: &Peertest) {
assert!(store_result);

let query_start_time = SystemTime::now();
let trace_content_info = HistoryNetworkApiClient::trace_recursive_find_content(
let trace_content_info = HistoryNetworkApiClient::trace_get_content(
&peertest.nodes[0].ipc_client,
content_key.clone(),
)
Expand Down Expand Up @@ -136,11 +136,11 @@ 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) {
pub async fn test_trace_get_content_for_absent_content(peertest: &Peertest) {
let client = &peertest.nodes[0].ipc_client;
let (content_key, _) = fixture_header_by_hash();

let error = HistoryNetworkApiClient::trace_recursive_find_content(client, content_key)
let error = HistoryNetworkApiClient::trace_get_content(client, content_key)
.await
.unwrap_err()
.to_string();
Expand All @@ -150,7 +150,7 @@ pub async fn test_trace_recursive_find_content_for_absent_content(peertest: &Pee
assert!(error.contains("-39001"));
}

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

let store_result = HistoryNetworkApiClient::store(
Expand All @@ -163,12 +163,10 @@ pub async fn test_trace_recursive_find_content_local_db(peertest: &Peertest) {

assert!(store_result);

let trace_content_info = HistoryNetworkApiClient::trace_recursive_find_content(
&peertest.bootnode.ipc_client,
content_key,
)
.await
.unwrap();
let trace_content_info =
HistoryNetworkApiClient::trace_get_content(&peertest.bootnode.ipc_client, content_key)
.await
.unwrap();
assert!(!trace_content_info.utp_transfer);
assert_eq!(trace_content_info.content, content_value.encode());

Expand Down
6 changes: 1 addition & 5 deletions ethportal-peertest/src/scenarios/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,7 @@ pub async fn test_gossip_dropped_with_find_content(peertest: &Peertest, target:
.unwrap();

// send find_content request from fresh target to target
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Unrelated to this pr, but this comment is wrong. Or at least, it looks like it was correct, but than line 301 was commented out and line 302 is added. Would be nice to fix it if you know what is happening here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it, that seems like an artifact left when writing the test

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment should still be updated to

let _result = fresh_target
//.find_content(target.node_info().await.unwrap().enr, acc_key_2.clone())
.recursive_find_content(body_key_2.clone())
.await
.unwrap();
let _result = fresh_target.get_content(body_key_2.clone()).await.unwrap();

// check that the fresh target has stored body_2 stored
assert_eq!(
Expand Down
4 changes: 2 additions & 2 deletions ethportal-peertest/src/scenarios/utp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub async fn test_recursive_utp(peertest: &Peertest) {

let content_info = peertest.nodes[0]
.ipc_client
.recursive_find_content(content_key)
.get_content(content_key)
.await
.unwrap();

Expand Down Expand Up @@ -76,7 +76,7 @@ pub async fn test_trace_recursive_utp(peertest: &Peertest) {

let trace_content_info: TraceContentInfo = peertest.nodes[0]
.ipc_client
.trace_recursive_find_content(content_key)
.trace_get_content(content_key)
.await
.unwrap();

Expand Down
19 changes: 5 additions & 14 deletions portal-bridge/src/bridge/era1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,7 @@ impl Era1Bridge {
let mut found = 0;
let hunter_threshold = (content_keys_to_sample.len() as u64 * threshold / 100) as usize;
for content_key in content_keys_to_sample {
let result = self
.portal_client
.recursive_find_content(content_key.clone())
.await;
let result = self.portal_client.get_content(content_key.clone()).await;
if let Ok(ContentInfo::Content { .. }) = result {
found += 1;
if found == hunter_threshold {
Expand Down Expand Up @@ -370,9 +367,7 @@ impl Era1Bridge {
if hunt {
let header_hash = block_tuple.header.header.hash();
let header_content_key = HistoryContentKey::new_block_header_by_hash(header_hash);
let header_content_info = portal_client
.recursive_find_content(header_content_key.clone())
.await;
let header_content_info = portal_client.get_content(header_content_key.clone()).await;
if let Ok(ContentInfo::Content { .. }) = header_content_info {
info!(
"Skipping header by hash at height: {} as header already found",
Expand Down Expand Up @@ -423,9 +418,7 @@ impl Era1Bridge {
if hunt {
let header_content_key =
HistoryContentKey::new_block_header_by_number(block_tuple.header.header.number);
let header_content_info = portal_client
.recursive_find_content(header_content_key.clone())
.await;
let header_content_info = portal_client.get_content(header_content_key.clone()).await;
if let Ok(ContentInfo::Content { .. }) = header_content_info {
info!(
"Skipping header by number at height: {} as header already found",
Expand Down Expand Up @@ -466,9 +459,7 @@ impl Era1Bridge {
if hunt {
let body_hash = block_tuple.header.header.hash();
let body_content_key = HistoryContentKey::new_block_body(body_hash);
let body_content_info = portal_client
.recursive_find_content(body_content_key.clone())
.await;
let body_content_info = portal_client.get_content(body_content_key.clone()).await;
if let Ok(ContentInfo::Content { .. }) = body_content_info {
info!(
"Skipping body at height: {} as body already found",
Expand Down Expand Up @@ -508,7 +499,7 @@ impl Era1Bridge {
let receipts_hash = block_tuple.header.header.hash();
let receipts_content_key = HistoryContentKey::new_block_receipts(receipts_hash);
let receipts_content_info = portal_client
.recursive_find_content(receipts_content_key.clone())
.get_content(receipts_content_key.clone())
.await;
if let Ok(ContentInfo::Content { .. }) = receipts_content_info {
info!(
Expand Down
6 changes: 2 additions & 4 deletions portal-bridge/src/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ async fn beacon_trace_gossip(
}
}
// if not, make rfc request to see if data is available on network
let result =
BeaconNetworkApiClient::recursive_find_content(&client, content_key.clone()).await;
let result = BeaconNetworkApiClient::get_content(&client, content_key.clone()).await;
if let Ok(ContentInfo::Content { .. }) = result {
debug!("Found content on network, after failing to gossip, aborting gossip. content key={:?}", content_key.to_hex());
found = true;
Expand Down Expand Up @@ -133,8 +132,7 @@ async fn history_trace_gossip(
}
}
// if not, make rfc request to see if data is available on network
let result =
HistoryNetworkApiClient::recursive_find_content(&client, content_key.clone()).await;
let result = HistoryNetworkApiClient::get_content(&client, content_key.clone()).await;
if let Ok(ContentInfo::Content { .. }) = result {
debug!("Found content on network, after failing to gossip, aborting gossip. content key={:?}", content_key.to_hex());
found = true;
Expand Down
17 changes: 8 additions & 9 deletions rpc/src/beacon_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,20 @@ impl BeaconNetworkApiServer for BeaconNetworkApi {
Ok(proxy_to_subnet(&self.network, endpoint).await?)
}

/// Lookup a target content key in the network
async fn recursive_find_content(
&self,
content_key: BeaconContentKey,
) -> RpcResult<ContentInfo> {
let endpoint = BeaconEndpoint::RecursiveFindContent(content_key);
/// First checks local storage if content is not found lookup a target content key in the
/// network
async fn get_content(&self, content_key: BeaconContentKey) -> RpcResult<ContentInfo> {
let endpoint = BeaconEndpoint::GetContent(content_key);
Ok(proxy_to_subnet(&self.network, endpoint).await?)
}

/// Lookup a target content key in the network. Return tracing info.
async fn trace_recursive_find_content(
/// First checks local storage if content is not found lookup a target content key in the
/// network. Return tracing info.
async fn trace_get_content(
&self,
content_key: BeaconContentKey,
) -> RpcResult<TraceContentInfo> {
let endpoint = BeaconEndpoint::TraceRecursiveFindContent(content_key);
let endpoint = BeaconEndpoint::TraceGetContent(content_key);
Ok(proxy_to_subnet(&self.network, endpoint).await?)
}

Expand Down
Loading
Loading