feat: implemented the 'Proof of Game' via SHA-256 Move History hashing feature while ensuring high resource efficiency (Gas/CPU) by utilizing a Rolling Hash#663
Conversation
…g feature while ensuring high resource efficiency (Gas/CPU) by utilizing a Rolling Hash
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 40 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/game_contract/src/lib.rs`:
- Line 155: Add unit tests in contracts/game_contract/src/lib.rs that assert the
new proof_of_game behavior: 1) test initial state uses BytesN::from_array(&env,
&[0; 32]) by instantiating the contract (constructor/new/instantiate) and
checking proof_of_game is zero; 2) test that calling submit_move(...) mutates
proof_of_game deterministically by calling submit_move once and asserting
proof_of_game changed from zero to a specific value (store that value from the
call); 3) test that two different move sequences produce different final
proof_of_game values by submitting two distinct sequences via submit_move and
comparing their proofs; and 4) test reproducibility by running the same sequence
twice (or reproducing the on-chain update logic off-chain using the same inputs)
and asserting the final proof_of_game values match; use the contract's getter or
direct state access for proof_of_game and reuse BytesN::from_array as the
expected zero-proof sentinel.
- Line 34: The Game struct literal in the tests is missing the new proof_of_game
field; update the Game literal to include proof_of_game initialized the same way
as in create_game by setting proof_of_game to BytesN::from_array(&env, &[0; 32])
so the test struct matches the Game definition (and the create_game
initialization).
- Around line 270-282: The proof payload is non-canonical: update the
construction around proof_payload/game.proof_of_game so each move is
length-prefixed and bound to the game and player; specifically, when appending
move_data (Vec<u32>) to proof_payload, first append its length as a fixed-size
integer (e.g., u32 LE), then append each u32 element in LE, and also include
chess_move.player (or equivalent author identifier) and game.id into the payload
before hashing; ensure you still include the prior game.proof_of_game and
timestamp (chess_move.timestamp) so the chain remains incremental and
domain-separated per game.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a1099a8-cdb4-4938-ac87-972649e46db4
📒 Files selected for processing (1)
contracts/game_contract/src/lib.rs
| pub moves: Vec<ChessMove>, | ||
| pub created_at: u64, | ||
| pub winner: Option<Address>, | ||
| pub proof_of_game: BytesN<32>, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Game struct literal constructions to ensure they include the new field.
rg -nP -U '(?s)\bGame\s*\{[^}]*\}' --type=rust -C2Repository: NOVUS-X/XLMate
Length of output: 10043
Update test.rs Game struct literal to include the required proof_of_game field.
The Game struct literal in test.rs (lines 23–33) is missing the proof_of_game: BytesN<32> field that was added to the struct definition. This will cause a compilation error. Add proof_of_game: BytesN::from_array(&env, &[0; 32]), to the literal to match the struct definition and the initialization used in create_game (lib.rs:155).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/game_contract/src/lib.rs` at line 34, The Game struct literal in
the tests is missing the new proof_of_game field; update the Game literal to
include proof_of_game initialized the same way as in create_game by setting
proof_of_game to BytesN::from_array(&env, &[0; 32]) so the test struct matches
the Game definition (and the create_game initialization).
| moves: Vec::new(&env), | ||
| created_at: env.ledger().sequence() as u64, | ||
| winner: None, | ||
| proof_of_game: BytesN::from_array(&env, &[0; 32]), |
There was a problem hiding this comment.
Missing unit tests for the new proof_of_game feature.
Issue #507 explicitly calls for unit tests covering standard and edge cases. The PR adds the field and update logic but no tests in this file verify: (a) the initial zero proof on creation, (b) that proof_of_game changes deterministically after each submit_move, (c) that different move sequences produce different proofs, and (d) that the same inputs reproduce the same final proof off-chain. Please add coverage.
Want me to draft the test cases?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/game_contract/src/lib.rs` at line 155, Add unit tests in
contracts/game_contract/src/lib.rs that assert the new proof_of_game behavior:
1) test initial state uses BytesN::from_array(&env, &[0; 32]) by instantiating
the contract (constructor/new/instantiate) and checking proof_of_game is zero;
2) test that calling submit_move(...) mutates proof_of_game deterministically by
calling submit_move once and asserting proof_of_game changed from zero to a
specific value (store that value from the call); 3) test that two different move
sequences produce different final proof_of_game values by submitting two
distinct sequences via submit_move and comparing their proofs; and 4) test
reproducibility by running the same sequence twice (or reproducing the on-chain
update logic off-chain using the same inputs) and asserting the final
proof_of_game values match; use the contract's getter or direct state access for
proof_of_game and reuse BytesN::from_array as the expected zero-proof sentinel.
|
|
||
| let mut proof_payload = Bytes::new(&env); | ||
| proof_payload.append(&game.proof_of_game.clone().into()); | ||
| for m in move_data.iter() { | ||
| proof_payload.append(&Bytes::from_slice(&env, &m.to_le_bytes())); | ||
| } | ||
| proof_payload.append(&Bytes::from_slice( | ||
| &env, | ||
| &chess_move.timestamp.to_le_bytes(), | ||
| )); | ||
| game.proof_of_game = env.crypto().sha256(&proof_payload).into(); | ||
|
|
||
| game.moves.push_back(chess_move); |
There was a problem hiding this comment.
Proof payload is not canonical — omits player/game_id and lacks length framing for move_data.
The current chaining hashes prev_proof || concat(move_data u32 LE) || timestamp_le. Two issues weaken the "proof of game":
- No length prefix for
move_data: sincemove_data: Vec<u32>has variable length and multiple moves are chained, concatenating raw bytes is ambiguous. A sequence of moves[1,2]then[3]produces the same concatenated bytes as[1]then[2,3]at the same timestamps — different histories can yield the same proof. - Missing binding fields: neither
player(move author) norgame.idis included. The chain can't attest who submitted each move, and proofs are not domain-separated per game.
Consider encoding the move with explicit framing, e.g.:
♻️ Proposed fix
let mut proof_payload = Bytes::new(&env);
proof_payload.append(&game.proof_of_game.clone().into());
+ // Domain separation / binding
+ proof_payload.append(&Bytes::from_slice(&env, &game.id.to_le_bytes()));
+ proof_payload.append(&Bytes::from_slice(&env, &(player_num as u32).to_le_bytes()));
+ // Length-prefix the variable-length move_data to prevent concatenation ambiguity
+ proof_payload.append(&Bytes::from_slice(&env, &(move_data.len() as u32).to_le_bytes()));
for m in move_data.iter() {
proof_payload.append(&Bytes::from_slice(&env, &m.to_le_bytes()));
}
proof_payload.append(&Bytes::from_slice(
&env,
&chess_move.timestamp.to_le_bytes(),
));
game.proof_of_game = env.crypto().sha256(&proof_payload).into();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/game_contract/src/lib.rs` around lines 270 - 282, The proof payload
is non-canonical: update the construction around
proof_payload/game.proof_of_game so each move is length-prefixed and bound to
the game and player; specifically, when appending move_data (Vec<u32>) to
proof_payload, first append its length as a fixed-size integer (e.g., u32 LE),
then append each u32 element in LE, and also include chess_move.player (or
equivalent author identifier) and game.id into the payload before hashing;
ensure you still include the prior game.proof_of_game and timestamp
(chess_move.timestamp) so the chain remains incremental and domain-separated per
game.
|
conflicts @CollinsC1O |
|
@CollinsC1O Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
resolved |
All Test do pass
|
2 participants
Implementation Details:
Added proof_of_game Field: The Game state struct now tracks an individual proof_of_game field of type BytesN<32>.
Initial State (Genesis Hash): When a game is successfully created via create_game, the proof_of_game initializes with a zeroed-out memory space BytesN::from_array(&env, &[0; 32]).
Optimized Move Settlement (O(1) Gas Costs): Instead of iterating through potentially hundreds of stored moves when a game concludes or requires verification, proof_of_game incrementally updates during every submit_move. It creates an env.crypto().sha256(payload) over (previous_proof_hash || move_data || timestamp).
Using an incremental cryptographic chaining strategy completely removes large allocation bottlenecks for huge dynamic arrays, securing standard operation against network limits or elevated CPU fees on Stellar and keeping node operations incredibly lightweight.
The underlying unit tests with the rust compilation were run locally and successfully passed!
Closes #507
Summary by CodeRabbit
Release Notes