feat: readme enhancing#100
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 26 minutes and 20 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesScript path consolidation and example-wide setup updates
pinocchio-private-counter permission model overhaul and legacy archival
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@00-LEGACY_EXAMPLES/pinocchio-private-counter/package.json`:
- Line 8: The relative paths in the npm scripts test:local (line 8), lint (line
16), and lint:fix (line 17) are incorrect because this is a nested legacy
package located two directories deep from the repo root. Currently these scripts
use ../scripts/ and ../node_modules/ paths, but from this nested location, the
repo root is at ../../, not ../. Update all three script definitions to use
../../scripts/ instead of ../scripts/ to correctly reference the scripts
directory and ensure the commands resolve to the correct paths.
In `@00-LEGACY_EXAMPLES/pinocchio-private-counter/README.md`:
- Around line 58-77: The Instructions section in the README.md file has markdown
linting issues with heading spacing. Each heading (### 0: InitializeCounter, ###
1: IncreaseCounter, ### 2: Delegate, ### 3: CommitAndUndelegate, ### 4: Commit,
### 5: IncrementAndCommit, and ### 6: IncrementAndUndelegate) requires blank
lines before and after it to comply with MD022 markdown linting rules. Add a
blank line before each heading and after each heading's content to fix the
linting violations.
In `@00-LEGACY_EXAMPLES/pinocchio-private-counter/src/processor.rs`:
- Around line 88-149: Add a critical authorization guard to enforce that only
the authority can invoke state-mutating handlers. At the start of
process_initialize_counter, add a check to verify that the authority_account is
a signer (using the authority_account.is_signer() check or equivalent).
Additionally, in process_initialize_counter, add a guard to reject
re-initialization by checking if counter_account.lamports() is greater than zero
and the counter is already initialized—return an error instead of allowing
reinitialization of existing state. Apply the same authority signer verification
guard at the start of the other affected functions: process_increase_counter
(lines 152-174), process_init_permission, process_set_privacy, and
process_close_permission (lines 229-381) to prevent unauthorized state mutations
across all mutating handlers.
In `@00-LEGACY_EXAMPLES/pinocchio-private-counter/src/state.rs`:
- Around line 19-21: Replace ProgramError::InvalidArgument with
ProgramError::InvalidAccountData in the validation check where the account
buffer size is compared to Self::SIZE. This error type is more semantically
correct for data-layout and deserialization failures rather than argument
validation failures. Apply this same change to all similar buffer size
validation checks mentioned in the comment (there are multiple locations with
this pattern).
In `@pinocchio-private-counter/package.json`:
- Around line 8-9: The test script on line 9 runs tests directly without
ensuring the setup/build phase has been completed first, causing immediate
failures on fresh checkouts or after clean builds due to missing artifacts like
target/deploy/pinocchio_private_counter-keypair.json. Modify the test script to
execute the setup script before running test:local, either by chaining commands
(e.g., yarn setup && yarn test:local) or using a yarn pretest lifecycle hook to
guarantee the build artifacts are created before tests run.
In `@pinocchio-private-counter/README.md`:
- Line 35: On line 35 of the README, replace the package name reference
`pinocchio-ephemeral-permission-counter` with `pinocchio-private-counter` in the
setup command example to match the actual package name defined in package.json
and ensure the documented command will work correctly.
In `@pinocchio-private-counter/src/entrypoint.rs`:
- Around line 107-109: The instruction_data length has already been validated to
be at least 8 bytes before reaching the slice operation on line 107. The use of
get(8..) followed by ok_or is redundant since the length check guarantees the
slice will succeed. Replace the safe checked slice operation with direct
indexing by changing the get(8..).ok_or(...) pattern to simply use
&instruction_data[8..], which is safe because the length has already been
validated.
In `@pinocchio-private-counter/src/processor.rs`:
- Around line 40-43: The idempotent check that returns early when an account
already exists and is owned by the program does not validate that the stored id
field matches the provided id parameter. This allows re-initialization with a
different id to silently succeed without any error or warning. After the
existing ownership check and before returning Ok(()), deserialize or read the
stored id from the counter account data using counter_info, compare it with the
provided id parameter, and either log a warning or return an error if they do
not match. Only return Ok(()) if both the account exists and ownership is
confirmed and the stored id matches the provided id.
- Around line 218-252: The function `process_update_permission` is missing
signer validation for the `payer_info` parameter that is used as the authority
in the UpdateEphemeralPermission invocation. Add a validation check at the
beginning of `process_update_permission` to verify that `payer_info.is_signer()`
returns true, returning `ProgramError::MissingRequiredSignature` if the check
fails. Apply the same validation to the `process_close_permission` function
which has the same issue where `payer_info` is passed as authority without
signer validation. This validation pattern is already present in
`process_commit_and_undelegate` and should be consistent across all functions
that use `payer_info` as a signer.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 371aa788-15c5-4762-ba8a-dde9d9ece257
⛔ Files ignored due to path filters (4)
00-LEGACY_EXAMPLES/pinocchio-private-counter/tests-rs/fixtures/dlp.sois excluded by!**/*.so00-LEGACY_EXAMPLES/pinocchio-private-counter/yarn.lockis excluded by!**/yarn.lock,!**/*.lockpinocchio-ephemeral-permission-counter/yarn.lockis excluded by!**/yarn.lock,!**/*.lockpinocchio-private-counter/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (85)
.github/workflows/test-examples.yml00-LEGACY_EXAMPLES/anchor-counter/Anchor.toml00-LEGACY_EXAMPLES/anchor-counter/package.json00-LEGACY_EXAMPLES/pinocchio-private-counter/.env.example00-LEGACY_EXAMPLES/pinocchio-private-counter/.gitignore00-LEGACY_EXAMPLES/pinocchio-private-counter/Cargo.toml00-LEGACY_EXAMPLES/pinocchio-private-counter/LICENSE00-LEGACY_EXAMPLES/pinocchio-private-counter/README.md00-LEGACY_EXAMPLES/pinocchio-private-counter/package.json00-LEGACY_EXAMPLES/pinocchio-private-counter/src/entrypoint.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/src/lib.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/src/processor.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/src/state.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/tests-rs/delegate_counter.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/tests-rs/increase_counter.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/tests-rs/initialize_counter.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/tests-rs/utils.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/initializeKeypair.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/kit/initializeKeypair.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/kit/pinocchio-private-counter.test.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/kit/schema.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/schema.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/web3js/initializeKeypair.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/web3js/pinocchio-private-counter.test.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tests/web3js/schema.ts00-LEGACY_EXAMPLES/pinocchio-private-counter/tsconfig.json00-LEGACY_EXAMPLES/pinocchio-private-counter/vitest.config.ts00-LEGACY_EXAMPLES/session-keys/Anchor.toml00-LEGACY_EXAMPLES/session-keys/package.json00-LEGACY_EXAMPLES/spl-tokens/package.jsonREADME.mdanchor-counter/README.mdanchor-counter/package.jsoncrank-counter/README.mdcrank-counter/package.jsondummy-token-transfer/README.mddummy-token-transfer/package.jsonephemeral-account-chats/README.mdephemeral-account-chats/package.jsonfullstack-test.shmagic-actions/README.mdmagic-actions/package.jsononcurve-delegation/README.mdoncurve-delegation/package.jsonpinocchio-counter/README.mdpinocchio-counter/package.jsonpinocchio-ephemeral-permission-counter/Cargo.tomlpinocchio-ephemeral-permission-counter/README.mdpinocchio-ephemeral-permission-counter/src/entrypoint.rspinocchio-ephemeral-permission-counter/src/processor.rspinocchio-ephemeral-permission-counter/src/state.rspinocchio-ephemeral-permission-counter/tsconfig.jsonpinocchio-private-counter/.gitignorepinocchio-private-counter/.yarnrc.ymlpinocchio-private-counter/Cargo.tomlpinocchio-private-counter/README.mdpinocchio-private-counter/package.jsonpinocchio-private-counter/src/entrypoint.rspinocchio-private-counter/src/processor.rspinocchio-private-counter/src/state.rspinocchio-private-counter/tests/pinocchio-private-counter.test.tspinocchio-private-counter/tsconfig.jsonpinocchio-roll-dice/README.mdpinocchio-roll-dice/package.jsonprivate-counter/README.mdprivate-counter/package.jsonrewards-delegated-vrf/README.mdrewards-delegated-vrf/package.jsonrock-paper-scissor/README.mdrock-paper-scissor/package.jsonrock-paper-scissor/programs/anchor-rock-paper-scissor/src/lib.rsroll-dice/README.mdroll-dice/package.jsonrust-counter/README.mdrust-counter/package.jsonscripts/check-prettier.shscripts/local-env.shscripts/projects.shscripts/test-example.shscripts/test-locally.shscripts/update-sdk-versions.shsession-keys/README.mdsession-keys/package.jsonspl-tokens/README.mdspl-tokens/package.json
💤 Files with no reviewable changes (9)
- pinocchio-ephemeral-permission-counter/README.md
- pinocchio-ephemeral-permission-counter/tsconfig.json
- 00-LEGACY_EXAMPLES/pinocchio-private-counter/.gitignore
- fullstack-test.sh
- pinocchio-ephemeral-permission-counter/Cargo.toml
- pinocchio-ephemeral-permission-counter/src/processor.rs
- 00-LEGACY_EXAMPLES/pinocchio-private-counter/src/lib.rs
- pinocchio-ephemeral-permission-counter/src/state.rs
- pinocchio-ephemeral-permission-counter/src/entrypoint.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
00-LEGACY_EXAMPLES/pinocchio-private-counter/src/processor.rs (1)
362-399:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing authority signer check in
process_close_permission.All other mutating handlers (
process_initialize_counter,process_increase_counter,process_delegate,process_init_permission,process_set_privacy,process_commit,process_commit_and_undelegate,process_increment_commit,process_increment_undelegate) now correctly enforcerequire_authority_signer, butprocess_close_permissiondoes not. This allows anyone who knows the authority pubkey and bump to close the ephemeral permission without authorization.🔒 Proposed fix
pub fn process_close_permission( program_id: &Address, accounts: &[AccountView], bump: u8, ) -> ProgramResult { let [authority, counter_account, permission, vault, magic_program, permission_program, ..] = accounts else { return Err(ProgramError::NotEnoughAccountKeys); }; + require_authority_signer(authority)?; + let counter_pda = counter_address_from_bump(program_id, authority.address(), bump)?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@00-LEGACY_EXAMPLES/pinocchio-private-counter/src/processor.rs` around lines 362 - 399, The `process_close_permission` function is missing authorization validation that all other mutating handlers implement. Add a require_authority_signer check at the beginning of `process_close_permission` to verify that the authority account is actually a signer before proceeding. Additionally, change `authority_is_signer: false` to `authority_is_signer: true` in the CloseEphemeralPermission struct instantiation to properly require the authority's signature when closing the ephemeral permission.README.md (1)
218-224:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExample workflow is missing the validator setup step.
Lines 218–224 show how to test
pinocchio-roll-dicewithyarn buildandyarn test:local, but this assumes the validators are already running from step 2 (yarn setup). Either addyarn setupin a separate terminal (as noted in step 2 above), or explicitly clarify that "this assumes validators from step 2 are running." Without validators,yarn test:localwill fail with RPC connection errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 218 - 224, The example workflow for testing pinocchio-roll-dice at lines 218–224 does not mention that validators must be running first (from step 2's `yarn setup` command), which will cause `yarn test:local` to fail with RPC connection errors. Either add a note to the example clarifying that validators from step 2 must be running in a separate terminal before executing the build and test commands, or insert `yarn setup` as a prerequisite step within the example itself. Choose the approach that best fits your documentation style and user expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@00-LEGACY_EXAMPLES/pinocchio-private-counter/src/processor.rs`:
- Around line 362-399: The `process_close_permission` function is missing
authorization validation that all other mutating handlers implement. Add a
require_authority_signer check at the beginning of `process_close_permission` to
verify that the authority account is actually a signer before proceeding.
Additionally, change `authority_is_signer: false` to `authority_is_signer: true`
in the CloseEphemeralPermission struct instantiation to properly require the
authority's signature when closing the ephemeral permission.
In `@README.md`:
- Around line 218-224: The example workflow for testing pinocchio-roll-dice at
lines 218–224 does not mention that validators must be running first (from step
2's `yarn setup` command), which will cause `yarn test:local` to fail with RPC
connection errors. Either add a note to the example clarifying that validators
from step 2 must be running in a separate terminal before executing the build
and test commands, or insert `yarn setup` as a prerequisite step within the
example itself. Choose the approach that best fits your documentation style and
user expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: da6d6500-c6fa-4fbc-8908-828e591ce118
📒 Files selected for processing (9)
00-LEGACY_EXAMPLES/pinocchio-private-counter/package.json00-LEGACY_EXAMPLES/pinocchio-private-counter/src/processor.rs00-LEGACY_EXAMPLES/pinocchio-private-counter/src/state.rsREADME.mdpinocchio-private-counter/README.mdpinocchio-private-counter/src/entrypoint.rspinocchio-roll-dice/package.jsonroll-dice/package.jsonscripts/test-locally.sh
Summary by CodeRabbit
scripts/test-locally.sh, removed legacy fullstack test utilities and redundant per-example test scripts, updated formatting check script paths, and removed thepinocchio-ephemeral-permission-counterexample materials.