feat:added changes to implement poolDistresult #1256
Conversation
Signed-off-by: Jenita <[email protected]>
|
Warning Rate limit exceeded@jkawan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a CLI "pool-distr" subcommand that requests pool distribution (invoked without explicit pool IDs), prints the raw response, and prints a JSON-serialized map of pool ID (hex) → entry. Changes the localstate query client Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🧹 Nitpick comments (1)
protocol/localstatequery/client.go (1)
839-856: Query construction for GetPoolDistr looks correct; consider tightening parameter typeThe conditional construction—no params for “all pools” and a
CborTagSetwhenpoolIdsis non-empty—matches the pattern used inGetStakePoolParamsand should be acceptable from a CBOR perspective.Two follow-ups worth considering:
- Use a stronger type for
poolIds(e.g.,[]ledger.PoolId) instead of[]anyfor consistency withGetStakePoolParamsand to prevent accidental misuse.- Double-check against the local-state-query CDDL/spec that the “no params” form for
QueryTypeShelleyPoolDistris indeed the correct way to request all pools.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/gouroboros/query.go(1 hunks)protocol/localstatequery/client.go(1 hunks)protocol/localstatequery/queries.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
protocol/localstatequery/client.go (3)
protocol/localstatequery/queries.go (1)
QueryTypeShelleyPoolDistr(64-64)cbor/cbor.go (1)
Tag(40-40)cbor/tags.go (1)
CborTagSet(31-31)
protocol/localstatequery/queries.go (3)
cbor/cbor.go (1)
StructAsArray(45-48)ledger/common/common.go (2)
PoolId(449-449)Blake2b256(41-41)cbor/tags.go (1)
Rat(94-96)
cmd/gouroboros/query.go (2)
protocol/localstatequery/localstatequery.go (1)
LocalStateQuery(116-119)protocol/localstatequery/client.go (1)
Client(30-43)
🪛 GitHub Actions: golangci-lint
cmd/gouroboros/query.go
[error] 330-330: encoding/json.Marshal for unsupported type github.com/blinklabs-io/gouroboros/ledger.PoolId as map key found (errchkjson)
🪛 GitHub Check: lint
cmd/gouroboros/query.go
[failure] 330-330:
encoding/json.Marshal for unsupported type github.com/blinklabs-io/gouroboros/ledger.PoolId as map key found (errchkjson)
⏰ 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: Analyze (go)
🔇 Additional comments (1)
protocol/localstatequery/queries.go (1)
725-734: PoolDistrResult structure is consistent with existing CBOR result typesThe layout mirrors
StakeDistributionResult(StructAsArray + map[ledger.PoolId]{StakeFraction, VrfHash}), which is appropriate for decoding the pool distribution response. No issues from a type/CBOR-shape perspective.
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
protocol/localstatequery/client.go
Outdated
| cbor.Tag{ | ||
| Number: cbor.CborTagSet, | ||
| Content: poolIds, | ||
| }, |
There was a problem hiding this comment.
We have the cbor.Set type that handles this
protocol/localstatequery/client.go
Outdated
| QueryTypeShelleyPoolDistr, | ||
| ) | ||
| } else { | ||
| query = buildShelleyQuery( |
There was a problem hiding this comment.
Instead of building the query twice, it's a bit cleaner to build the params dynamically. Something like this:
var params []any
if len(poolIds) > 0 {
params = append(params, cbor.Set(poolIds))
}
query := buildShelleyQuery(
currentEra,
QueryTypeShelleyPoolDistr,
params...,
)
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="cmd/gouroboros/query.go">
<violation number="1" location="cmd/gouroboros/query.go:339">
P2: JSON marshaling error handling is inconsistent with other cases in this file. Other cases (e.g., `utxos-by-address`, `utxos-by-txin`) call `os.Exit(1)` after JSON marshal failures and use `ERROR:` prefix format. This case silently continues execution.</violation>
</file>
<file name="protocol/localstatequery/client.go">
<violation number="1" location="protocol/localstatequery/client.go:840">
P3: The comment is misleading: 'tag=21' refers to the query type, not a CBOR tag. The CBOR set tag is actually 258 (`cbor.CborTagSet`). Consider clarifying: 'The query expects format: [queryType=21, CborTagSet(258, poolIds)]' to avoid confusion between query types and CBOR tags.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| } | ||
| jsonData, err := json.Marshal(jsonResults) | ||
| if err != nil { | ||
| fmt.Printf("pool-distr (JSON marshaling failed): %s\n", err) |
There was a problem hiding this comment.
P2: JSON marshaling error handling is inconsistent with other cases in this file. Other cases (e.g., utxos-by-address, utxos-by-txin) call os.Exit(1) after JSON marshal failures and use ERROR: prefix format. This case silently continues execution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/gouroboros/query.go, line 339:
<comment>JSON marshaling error handling is inconsistent with other cases in this file. Other cases (e.g., `utxos-by-address`, `utxos-by-txin`) call `os.Exit(1)` after JSON marshal failures and use `ERROR:` prefix format. This case silently continues execution.</comment>
<file context>
@@ -315,6 +315,31 @@ func testQuery(f *globalFlags) {
+ }
+ jsonData, err := json.Marshal(jsonResults)
+ if err != nil {
+ fmt.Printf("pool-distr (JSON marshaling failed): %s\n", err)
+ } else {
+ fmt.Printf("pool-distr (JSON): %s\n", string(jsonData))
</file context>
| fmt.Printf("pool-distr (JSON marshaling failed): %s\n", err) | |
| fmt.Printf("ERROR: failed to marshal pool-distr JSON: %s\n", err) | |
| os.Exit(1) |
| return nil, err | ||
| } | ||
| // GetPoolDistr always requires a pool set parameter according to the Haskell implementation | ||
| // The query expects (len=2, tag=21) format: [21, Set(poolIds)] |
There was a problem hiding this comment.
P3: The comment is misleading: 'tag=21' refers to the query type, not a CBOR tag. The CBOR set tag is actually 258 (cbor.CborTagSet). Consider clarifying: 'The query expects format: [queryType=21, CborTagSet(258, poolIds)]' to avoid confusion between query types and CBOR tags.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At protocol/localstatequery/client.go, line 840:
<comment>The comment is misleading: 'tag=21' refers to the query type, not a CBOR tag. The CBOR set tag is actually 258 (`cbor.CborTagSet`). Consider clarifying: 'The query expects format: [queryType=21, CborTagSet(258, poolIds)]' to avoid confusion between query types and CBOR tags.</comment>
<file context>
@@ -836,10 +836,19 @@ func (c *Client) GetPoolDistr(poolIds []any) (*PoolDistrResult, error) {
return nil, err
}
+ // GetPoolDistr always requires a pool set parameter according to the Haskell implementation
+ // The query expects (len=2, tag=21) format: [21, Set(poolIds)]
+ // If no pool IDs specified, use an empty set to query all pools
+ if poolIds == nil {
</file context>
| // The query expects (len=2, tag=21) format: [21, Set(poolIds)] | |
| // The query format is: [queryType(21), CborTagSet(258, poolIds)] |
Closes #870
While running the query against the cardano node, I keep getting one of the following
root@7ebe79e76ec2:/code# go run ./cmd/gouroboros/ -network devnet -network-magic 42 -socket /ipc/node.socket query pool-distr
ERROR: peer closed the connection while reading header: EOF
exit status 1
root@7ebe79e76ec2:/code# go run ./cmd/gouroboros/ -network devnet -network-magic 42 -socket /ipc/node.socket query pool-distr
ERROR: failure querying pool distribution: protocol is shutting down
exit status 1
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.