-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add get_scores
view function
#175
Conversation
WalkthroughThe pull request updates the VIP score module by introducing a constant, two new structures, and a public view function. A constant Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as VIPScoreModule
participant S as ModuleStore
U ->> V: call get_scores(stage, limit, start_after)
V ->> S: fetch scores for specified stage
S -->> V: return score records
V ->> V: enforce MAX_LIMIT and filter scores
V -->> U: return GetScoresResponse with stage and scores
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
precompile/modules/minitia_stdlib/sources/vip/score.move (2)
177-211
: Add documentation for the view function parameters and return value.The function implementation is solid, but it lacks documentation explaining:
- The meaning and valid ranges for the
stage
parameter- The purpose and behavior of the
limit
parameter- The role of the
start_after
parameter in pagination- The structure and interpretation of the return value
Add this documentation above the function:
+/// Returns a paginated list of scores for the specified stage. +/// @param stage: The stage number to query scores for +/// @param limit: Maximum number of scores to return (capped at MAX_LIMIT) +/// @param start_after: Optional address to start pagination after (exclusive) +/// @return GetScoresResponse containing the stage and vector of Score entries #[view] public fun get_scores(
196-201
: Fix the hardcoded iterator direction.The table iterator's direction parameter is hardcoded to 2. This should use a named constant for better readability and maintainability.
Consider adding a constant and using it:
+ const ASCENDING: u8 = 2; + let iter = table::iter( &score.score, option::none(), start_after, // exclusive - 2 + ASCENDING );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
precompile/modules/minitia_stdlib/sources/vip/score.move
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Rust libmovevm
🔇 Additional comments (1)
precompile/modules/minitia_stdlib/sources/vip/score.move (1)
24-28
: LGTM! Well-structured data model with appropriate limits.The MAX_LIMIT constant and the new structs provide a clean interface for paginated score retrieval while preventing potential DoS attacks.
Also applies to: 167-175
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
precompile/modules/minitia_stdlib/sources/vip/score.move
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Rust libmovevm
🔇 Additional comments (3)
precompile/modules/minitia_stdlib/sources/vip/score.move (3)
24-28
: LGTM! The MAX_LIMIT constant is well-defined.The constant value of 1000 is a reasonable limit for pagination to prevent excessive resource consumption.
167-175
: LGTM! The response structs are well-designed.The
GetScoresResponse
andScore
structs provide a clean interface for returning paginated score data.
177-218
: 🏁 Script executed:#!/bin/bash # Search for existing test cases that verify pagination rg -A 5 "get_scores.*test"Length of output: 28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
To support variable scores without state changes, the
get_scores
interface has been added. This allows the VIP agent to call the view function to retrieve scores instead of directly reading the table.The L2 operator is permitted to modify the
get_scores
function, provided the following principles are followed:Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit