feat: add jsonrpc prt tests #733

mpolitzer · 2026-01-27T18:18:44Z

closes #682

Copilot

Pull request overview

This PR expands JSON-RPC test coverage for tournament-related (“prt”) methods and adjusts repository/query behavior to make list endpoints deterministic and align empty-list responses with existing “Application not found” semantics.

Changes:

Add extensive JSON-RPC tests for tournament/commitment/match/match-advanced endpoints.
Make list endpoints return RESOURCE_NOT_FOUND: Application not found when the result is empty and the application does not exist.
Stabilize ordering for pagination in Postgres queries (commitments and match advances).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
internal/repository/postgres/match_advanced.go	Changes ordering for `ListMatchAdvances` to sort by `OtherParent` (stabilizes pagination within a fixed epoch/tournament/id_hash).
internal/repository/postgres/commitment.go	Adds secondary ORDER BY on `Commitment` to make pagination deterministic when many rows share the same `EpochIndex`.
internal/jsonrpc/jsonrpc.go	Adds “application exists” checks for multiple list handlers; tweaks `GetMatchAdvanced` parent handling.
internal/jsonrpc/util_test.go	Adds helper result structs for JSON unmarshalling in tests.
internal/jsonrpc/jsonrpc_test.go	Adds large suite of tests for new/expanded tournament data methods.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/jsonrpc/jsonrpc_test.go

internal/jsonrpc/jsonrpc.go

internal/jsonrpc/util_test.go

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T18:29:40Z

internal/jsonrpc/jsonrpc.go

+	if len(tournaments) == 0 && !s.applicationExists(w, r, req, params.Application) {
+		writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)
+		return
+	}


applicationExists writes an INTERNAL_ERROR response on repository failure and returns false. In this handler, that false result then triggers a second write of a RESOURCE_NOT_FOUND ("Application not found"), which can lead to double-writes and masking real DB errors. Consider changing applicationExists to return (bool, error) (or to never write to w) so the caller can return immediately on error and only emit "Application not found" when the check succeeds and the app truly doesn't exist.

Copilot · 2026-02-12T18:29:41Z

internal/jsonrpc/jsonrpc.go

+	if len(commitments) == 0 && !s.applicationExists(w, r, req, params.Application) {
+		writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)
+		return
+	}


applicationExists can write an INTERNAL_ERROR response and return false; this handler then unconditionally writes "Application not found" when it sees false, which risks double responses and incorrect error codes on transient repository failures. Refactor the existence check to return an error (or avoid writing to the ResponseWriter) and have the caller return immediately when that error occurs.

Copilot · 2026-02-12T18:29:41Z

internal/jsonrpc/jsonrpc.go

 		writeRPCError(w, req.ID, JSONRPC_INTERNAL_ERROR, "Internal server error", nil)
 		return
 	}
+	if len(matches) == 0 && !s.applicationExists(w, r, req, params.Application) {


This new applicationExists check can emit two responses if GetApplication fails: applicationExists writes INTERNAL_ERROR and returns false, then this branch writes RESOURCE_NOT_FOUND. Please change the helper to return an error (and not write), and handle that error here with an early return.

Suggested change

if len(matches) == 0 && !s.applicationExists(w, r, req, params.Application) {

if len(matches) == 0 {

if !s.applicationExists(w, r, req, params.Application) {

// applicationExists may have already written an error response; avoid double writes

return

}

Copilot · 2026-02-12T18:29:41Z

internal/jsonrpc/jsonrpc.go

+	if len(matchAdvances) == 0 && !s.applicationExists(w, r, req, params.Application) {
+		writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)
+		return


If applicationExists hits a repository error, it currently writes an INTERNAL_ERROR and returns false; this code then also writes "Application not found", resulting in double-writes and hiding the underlying failure. Adjust the helper to return (exists bool, err error) (or never write to w) and handle errors explicitly before emitting RESOURCE_NOT_FOUND.

Suggested change

if len(matchAdvances) == 0 && !s.applicationExists(w, r, req, params.Application) {

writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)

return

if len(matchAdvances) == 0 {

exists, err := s.applicationExists(r.Context(), params.Application)

if err != nil {

s.Logger.Error("Unable to check application existence", "err", err)

writeRPCError(w, req.ID, JSONRPC_INTERNAL_ERROR, "Internal server error", nil)

return

}

if !exists {

writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)

return

}

Copilot · 2026-02-12T18:29:42Z

internal/jsonrpc/jsonrpc_test.go

+			body := s.doRequest(t, 0, fmt.Appendf([]byte{}, `{
+				"jsonrpc": "2.0",
+				"method": "cartesi_getCommitment",
+				"params": {
+				"application": "%v",
+				"epoch_index": "%v",
+				"tournament_address": "0x%020x"
+				},
+				"id": 0
+				}`, numberToName(app), hexutil.EncodeUint64(nr+1), 0))


This "absent" test case for cartesi_getCommitment omits the required commitment parameter. Because the handler's validation currently treats an empty string as valid hex (hex.DecodeString("") succeeds), this test may pass for the wrong reason and doesn't cover the intended contract. Include an explicit commitment value for the not-found scenario, and add a separate test that verifies missing commitment returns JSONRPC_INVALID_PARAMS (if that's the desired API behavior).

mpolitzer requested review from renatomaia and vfusco January 27, 2026 18:18

mpolitzer self-assigned this Jan 27, 2026

mpolitzer added the #feat:tests Feature: tests label Jan 27, 2026

mpolitzer added this to Node Unit Jan 27, 2026

mpolitzer force-pushed the feature/add-jsonrpc-prt-tests branch from 5b26aa7 to 875aa23 Compare January 27, 2026 18:19

vfusco requested review from Copilot and removed request for renatomaia and vfusco February 11, 2026 12:35

Copilot started reviewing on behalf of vfusco February 11, 2026 12:35 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

mpolitzer force-pushed the feature/add-jsonrpc-prt-tests branch 2 times, most recently from 5442605 to e0b5bff Compare February 11, 2026 19:17

This was linked to issues Feb 12, 2026

Tests: JSON-RPC correctness, limit & security coverage, and OpenRPC schema compliance #682

Open

Add unit tests for new JSON-RPC methods (PRT dispute entities) #726

Open

mpolitzer moved this to 🏗 In progress in Node Unit Feb 12, 2026

vfusco requested a review from Copilot February 12, 2026 18:23

Copilot started reviewing on behalf of vfusco February 12, 2026 18:24 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

mpolitzer force-pushed the feature/add-jsonrpc-prt-tests branch from e0b5bff to a13fa56 Compare February 12, 2026 18:49

feat: add jsonrpc prt tests

290a589

mpolitzer force-pushed the feature/add-jsonrpc-prt-tests branch from a13fa56 to 290a589 Compare February 12, 2026 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add jsonrpc prt tests #733

feat: add jsonrpc prt tests #733

Uh oh!

mpolitzer commented Jan 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 12, 2026

Uh oh!

Copilot AI Feb 12, 2026

Uh oh!

Copilot AI Feb 12, 2026

Uh oh!

Copilot AI Feb 12, 2026

Uh oh!

Copilot AI Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-	if len(matches) == 0 && !s.applicationExists(w, r, req, params.Application) {
+	if len(matches) == 0 {
+		if !s.applicationExists(w, r, req, params.Application) {
+			// applicationExists may have already written an error response; avoid double writes
+			return
+		}

-	if len(matchAdvances) == 0 && !s.applicationExists(w, r, req, params.Application) {
-		writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)
-		return
+	if len(matchAdvances) == 0 {
+		exists, err := s.applicationExists(r.Context(), params.Application)
+		if err != nil {
+			s.Logger.Error("Unable to check application existence", "err", err)
+			writeRPCError(w, req.ID, JSONRPC_INTERNAL_ERROR, "Internal server error", nil)
+			return
+		}
+		if !exists {
+			writeRPCError(w, req.ID, JSONRPC_RESOURCE_NOT_FOUND, "Application not found", nil)
+			return
+		}

feat: add jsonrpc prt tests #733

Are you sure you want to change the base?

feat: add jsonrpc prt tests #733

Uh oh!

Conversation

mpolitzer commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mpolitzer commented Jan 27, 2026 •

edited

Loading