Add new query for fetching all metaboard addresses #84
Conversation
WalkthroughAdds a new subgraph query to fetch MetaBoard addresses, introduces corresponding GraphQL types, extends the metaboard client with an async method to return Vec , and adds error variants for request and address parsing failures.Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant S as MetaboardSubgraphClient
participant G as Subgraph API
participant P as Address Parser
C->>S: get_metaboard_addresses(first, skip)
S->>G: Query meta_boards(first, skip)
G-->>S: meta_boards [{ address: "0x..." }, ...] or error
alt Success
loop For each board
S->>P: parse board.address
P-->>S: Address or parse error
end
alt Any parse error
S-->>C: Err(AddressParseError)
else All parsed
S-->>C: Ok(Vec<Address>)
end
else Request error
S-->>C: Err(RequestErrorMetaBoards)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
crates/metaboard/src/metaboard_client.rs(4 hunks)crates/metaboard/src/types/metas.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-test)
🔇 Additional comments (4)
crates/metaboard/src/metaboard_client.rs (2)
3-7: LGTM!The imports correctly extend the existing alloy primitives usage to include
Addressand the necessaryFromStrtrait for address parsing.
33-43: LGTM!The new error variants follow the existing error handling patterns in the codebase. They provide clear error messages with appropriate context and properly chain source errors using the
#[source]attribute.crates/metaboard/src/types/metas.rs (2)
26-30: LGTM!The
MetaBoardAddressesVariablesstruct correctly defines pagination parameters following GraphQL and cynic conventions. TheDefaultderive is helpful for queries without pagination.
32-37: LGTM!The
MetaBoardAddressesquery fragment is correctly structured, properly references the variables struct, and correctly maps the pagination arguments. It reuses the existingMetaBoardtype appropriately.
| /// Fetch MetaBoard contract addresses from the subgraph. | ||
| pub async fn get_metaboard_addresses( | ||
| &self, | ||
| first: Option<i32>, | ||
| skip: Option<i32>, | ||
| ) -> Result<Vec<Address>, MetaboardSubgraphClientError> { | ||
| let data = | ||
| self.query::<MetaBoardAddresses, MetaBoardAddressesVariables>( | ||
| MetaBoardAddressesVariables { first, skip }, | ||
| ) | ||
| .await | ||
| .map_err(|e| MetaboardSubgraphClientError::RequestErrorMetaBoards { source: e })?; | ||
|
|
||
| let mut addresses = Vec::with_capacity(data.meta_boards.len()); | ||
| for board in data.meta_boards { | ||
| let address_hex = board.address.0; | ||
| let address = Address::from_str(&address_hex).map_err(|e| { | ||
| MetaboardSubgraphClientError::AddressParseError { | ||
| address: address_hex.clone(), | ||
| source: e, | ||
| } | ||
| })?; | ||
|
|
||
| addresses.push(address); | ||
| } | ||
|
|
||
| Ok(addresses) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify empty result handling is intentional.
The implementation correctly parses addresses and handles errors. However, there's an inconsistency with the existing methods:
get_metabytes_by_hashandget_metabytes_by_subjectreturnMetaboardSubgraphClientError::Emptywhen no results are found- This method returns an empty
Vec<Address>when no results are found
Confirm whether returning an empty vector for zero MetaBoards is the intended behavior, or if it should follow the existing pattern and return an error.
Optional: Consider validating pagination parameters.
The pagination parameters first and skip are passed directly to the subgraph without validation. While the subgraph will likely handle invalid values, client-side validation (e.g., rejecting negative values) could provide clearer error messages.
🤖 Prompt for AI Agents
crates/metaboard/src/metaboard_client.rs lines 130-157: currently
get_metaboard_addresses returns an empty Vec when the subgraph returns zero
MetaBoards, which is inconsistent with other methods that return
MetaboardSubgraphClientError::Empty; change the method to check if
data.meta_boards.is_empty() and return Err(MetaboardSubgraphClientError::Empty)
to match existing behavior (or if empty Vec is intended, add a clear comment and
update other methods for consistency), and optionally add simple validation to
reject negative pagination parameters (first, skip) and return a descriptive
error before making the query.
There was a problem hiding this comment.
an empty array is also fine. this means nothing is found
There was a problem hiding this comment.
@findolor, understood! I've noted that returning an empty array is the intended behavior for get_metaboard_addresses. The method will remain as-is.
/\_/\
( o.o )
> ^ <
/| |_
( === _)
`-.-'
✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/rain.metadata#84
File: crates/metaboard/src/metaboard_client.rs:130-157
Timestamp: 2025-10-02T12:33:46.451Z
Learning: In `crates/metaboard/src/metaboard_client.rs`, the `get_metaboard_addresses` method intentionally returns an empty `Vec<Address>` when no MetaBoards are found, rather than returning a `MetaboardSubgraphClientError::Empty` error like other methods in the same file. This design choice is acceptable and means nothing was found.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| #[tokio::test] | ||
| async fn test_get_metaboard_addresses_success() { | ||
| let server = MockServer::start_async().await; | ||
| let url = Url::parse(&server.url("/")).unwrap(); | ||
|
|
||
| server.mock(|when, then| { | ||
| when.method(POST).path("/").body_contains("metaBoards"); | ||
| then.status(200).json_body_obj(&{ | ||
| serde_json::json!({ | ||
| "data": { | ||
| "metaBoards": [ | ||
| { | ||
| "address": "0x0000000000000000000000000000000000000001", | ||
| }, | ||
| { | ||
| "address": "0x0000000000000000000000000000000000000002", | ||
| } | ||
| ] | ||
| } | ||
| }) | ||
| }); | ||
| }); | ||
|
|
||
| let client = MetaboardSubgraphClient::new(url); | ||
|
|
||
| let result = client | ||
| .get_metaboard_addresses(Some(10), Some(0)) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(result.len(), 2); | ||
|
|
||
| assert_eq!( | ||
| result[0], | ||
| Address::from_str("0x0000000000000000000000000000000000000001").unwrap() | ||
| ); | ||
| assert_eq!( | ||
| result[1], | ||
| Address::from_str("0x0000000000000000000000000000000000000002").unwrap() | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_get_metaboard_addresses_empty() { | ||
| let server = MockServer::start_async().await; | ||
| let url = Url::parse(&server.url("/")).unwrap(); | ||
|
|
||
| server.mock(|when, then| { | ||
| when.method(POST).path("/").body_contains("metaBoards"); | ||
| then.status(200).json_body_obj(&{ | ||
| serde_json::json!({ | ||
| "data": { | ||
| "metaBoards": [] | ||
| } | ||
| }) | ||
| }); | ||
| }); | ||
|
|
||
| let client = MetaboardSubgraphClient::new(url); | ||
|
|
||
| let result = client.get_metaboard_addresses(Some(5), None).await.unwrap(); | ||
|
|
||
| assert!(result.is_empty()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Consider adding error path tests.
The success and empty result tests are well-structured and follow the existing test patterns. However, consider adding tests for error paths to improve coverage:
- Invalid address format (should return
AddressParseError) - Request failure (should return
RequestErrorMetaBoards)
Example test structure:
#[tokio::test]
async fn test_get_metaboard_addresses_invalid_address() {
let server = MockServer::start_async().await;
let url = Url::parse(&server.url("/")).unwrap();
server.mock(|when, then| {
when.method(POST).path("/").body_contains("metaBoards");
then.status(200).json_body_obj(&{
serde_json::json!({
"data": {
"metaBoards": [
{
"address": "invalid_address",
}
]
}
})
});
});
let client = MetaboardSubgraphClient::new(url);
let result = client.get_metaboard_addresses(None, None).await;
assert!(result.is_err());
match result {
Err(MetaboardSubgraphClientError::AddressParseError { .. }) => (),
_ => panic!("Expected AddressParseError, got: {:?}", result),
}
}🤖 Prompt for AI Agents
In crates/metaboard/src/metaboard_client.rs around lines 337-400, add missing
error-path tests for get_metaboard_addresses: create one async test where the
mock returns a 200 JSON with an invalid address string and assert the call
returns Err(MetaboardSubgraphClientError::AddressParseError { .. }), and create
another async test where the mock returns a non-200 status (or triggers a
request failure) and assert it returns
Err(MetaboardSubgraphClientError::RequestErrorMetaBoards { .. }); ensure tests
use MockServer::start_async(), build the client with the mock URL, call
get_metaboard_addresses, and pattern-match the expected error variants.
There was a problem hiding this comment.
this is fine as contracts are indexed in sg. we are getting proper addresses
There was a problem hiding this comment.
@findolor, understood! Since the subgraph ensures valid addresses are indexed, the existing success and empty result tests provide sufficient coverage. Thank you for clarifying! 👍
🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/rain.metadata#84
File: crates/metaboard/src/metaboard_client.rs:130-157
Timestamp: 2025-10-02T12:33:46.451Z
Learning: In `crates/metaboard/src/metaboard_client.rs`, the `get_metaboard_addresses` method intentionally returns an empty `Vec<Address>` when no MetaBoards are found, rather than returning a `MetaboardSubgraphClientError::Empty` error like other methods in the same file. This design choice is acceptable and means nothing was found.
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
The author of this PR is on the CodeRabbit Free Plan. In order to use the Chat feature, please upgrade the PR author to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. |
Motivation
We needed a way to fetch all of the contract addresses from subgraph. This PR adds that.
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit