Conversation
efdc751 to
305b9fd
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds CLI support for reading PRT (Permissionless Referee Tournament) dispute entities (tournaments, commitments, matches, match_advances) to align with the JSON-RPC API. It also refactors existing read commands for consistency and improves the repository/JSON-RPC layers by changing filter field types from strings to proper address types.
Changes:
- Added four new CLI read commands for PRT dispute entities (tournaments, commitments, matches, match_advances)
- Refactored existing read commands (epochs, inputs, outputs, reports) for consistency in structure and error handling
- Changed
CommitmentFilter.TournamentAddressandMatchFilter.TournamentAddressfrom*stringto*common.Addressin the repository layer - Renamed
GetProcessedInputstoGetProcessedInputCountin the repository interface - Exported
parseIndexasParseIndexin JSON-RPC for broader use - Fixed JSON field naming:
IDHash→IdHashto match OpenRPC spec (snake_caseid_hash)
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 31 comments.
Show a summary per file
| File | Description |
|---|---|
internal/repository/repository.go |
Changed filter field types to use *common.Address instead of *string; renamed method to GetProcessedInputCount |
internal/repository/postgres/match.go |
Updated to use common.Address type directly from filter, removing intermediate conversion |
internal/repository/postgres/commitment.go |
Updated to use common.Address type directly from filter, removing intermediate conversion |
internal/repository/postgres/application.go |
Renamed GetProcessedInputs to GetProcessedInputCount |
internal/jsonrpc/types.go |
Fixed JSON field names from IDHash to IdHash to match OpenRPC spec |
internal/jsonrpc/jsonrpc.go |
Exported parseIndex as ParseIndex; updated filter handling to parse addresses before assigning to filters; updated all references to use new field names |
cmd/cartesi-rollups-cli/root/read/tournaments/tournaments.go |
New CLI command for reading tournaments |
cmd/cartesi-rollups-cli/root/read/commitments/commitments.go |
New CLI command for reading commitments |
cmd/cartesi-rollups-cli/root/read/matches/matches.go |
New CLI command for reading matches |
cmd/cartesi-rollups-cli/root/read/match_advances/match_advances.go |
New CLI command for reading match advances |
cmd/cartesi-rollups-cli/root/read/read.go |
Wired new commands into the CLI |
cmd/cartesi-rollups-cli/root/read/epochs/epochs.go |
Refactored for consistency with new command structure |
cmd/cartesi-rollups-cli/root/read/inputs/inputs.go |
Refactored for consistency with new command structure |
cmd/cartesi-rollups-cli/root/read/outputs/outputs.go |
Refactored for consistency with new command structure |
cmd/cartesi-rollups-cli/root/read/reports/reports.go |
Refactored for consistency with new command structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil { | ||
| cobra.CheckErr(fmt.Errorf("invalid epoch index value: %w", err)) | ||
| } | ||
|
|
There was a problem hiding this comment.
The tournament address argument (args[2]) and ID hash argument (args[3]) should be validated before being passed to the repository. The JSON-RPC handler validates the tournament address using config.ToAddressFromString and validates the ID hash using config.ToHashFromString. Add similar validation here.
| // Validate tournament address and id hash arguments | |
| if _, err := config.ToAddressFromString(args[2]); err != nil { | |
| cobra.CheckErr(fmt.Errorf("invalid tournament address value: %w", err)) | |
| } | |
| if _, err := config.ToHashFromString(args[3]); err != nil { | |
| cobra.CheckErr(fmt.Errorf("invalid id hash value: %w", err)) | |
| } |
cmd/cartesi-rollups-cli/root/read/match_advances/match_advances.go
Outdated
Show resolved
Hide resolved
| if err != nil { | ||
| cobra.CheckErr(fmt.Errorf("invalid application value: %w", err)) | ||
| } | ||
|
|
There was a problem hiding this comment.
The tournament address argument (args[1]) should be validated before being passed to the repository. The JSON-RPC handler validates this using config.ToAddressFromString. Add validation similar to line 131 where the flag value is validated.
| if _, err := config.ToAddressFromString(args[1]); err != nil { | |
| cobra.CheckErr(fmt.Errorf("invalid tournament address value: %w", err)) | |
| } |
e84cea7 to
1b7160a
Compare
d93c155 to
ac88e77
Compare
ac88e77 to
90c3e94
Compare
mpolitzer
left a comment
There was a problem hiding this comment.
Raised a few nitpicks, looking great other than that.
| ) | ||
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "commitments [application-name-or-address] [epoch-index] [tournament-address] [commitment-hash]", |
There was a problem hiding this comment.
| Use: "commitments [application-name-or-address] [epoch-index] [tournament-address] [commitment-hash]", | |
| Use: "commitments <application-name-or-address> [epoch-index] [tournament-address] [commitment-hash]", |
| Offset: offset, | ||
| }, false) | ||
| cobra.CheckErr(err) | ||
|
|
There was a problem hiding this comment.
Should we check if application exists in case of an len(commitments) == 0 like we do in the jsonrpc?
Same for other "list" like commands.
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "epochs [application-name-or-address]", | ||
| Use: "epochs [application-name-or-address] [epoch-index]", |
There was a problem hiding this comment.
| Use: "epochs [application-name-or-address] [epoch-index]", | |
| Use: "epochs <application-name-or-address> [epoch-index]", |
| ) | ||
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "matches [application-name-or-address] [epoch-index] [tournament-address] [id-hash]", |
There was a problem hiding this comment.
| Use: "matches [application-name-or-address] [epoch-index] [tournament-address] [id-hash]", | |
| Use: "matches <application-name-or-address> [epoch-index] [tournament-address] [id-hash]", |
| ) | ||
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "reports [application-name-or-address] [report-index]", |
There was a problem hiding this comment.
| Use: "reports <application-name-or-address> [report-index]", |
| ) | ||
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "outputs [application-name-or-address] [output-index]", |
There was a problem hiding this comment.
| Use: "outputs <application-name-or-address> [output-index]", |
| ) | ||
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "inputs [application-name-or-address] [input-index]", |
There was a problem hiding this comment.
| Use: "inputs <application-name-or-address> [input-index]", |
| ) | ||
|
|
||
| var Cmd = &cobra.Command{ | ||
| Use: "tournaments [application-name-or-address] [tournament-address]", |
There was a problem hiding this comment.
| Use: "tournaments [application-name-or-address] [tournament-address]", | |
| Use: "tournaments <application-name-or-address> [tournament-address]", |
closes #722