feat: created template for magic block's private and public counter using privy for wallet connections and temp keypair#88
Open
datmedevil17 wants to merge 2 commits into
Open
Conversation
…tilizing privy instead of temp keypair
|
@datmedevil17 is attempting to deploy a commit to the MagicBlock Labs Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Warning Review limit reached
More reviews will be available in 46 minutes and 45 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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)
WalkthroughThis PR introduces ChangesAnchor Counter Privy Application
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 29
🤖 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 `@anchor-counter-privy/.claude/settings.json`:
- Around line 4-10: The allowlist contains overly-broad wildcard Bash entries
like "Bash(yarn add *)", "Bash(npx cross-env *)" and similar patterns that
permit arbitrary command execution; replace those wildcard entries with a
minimal, explicit set of safe inspection commands or remove them entirely (for
example, keep the specific node/python inspection lines such as the existing
"Bash(node -e ...)" and "Bash(python3 -c ...)" entries) and instead whitelist
exact invocations or read-only flags (no "*" wildcards) for symbols like
"Bash(yarn add *)", "Bash(npm list *)", and "Bash(npx cross-env *)" so the agent
can only run predefined, read-only commands.
In `@anchor-counter-privy/.gitignore`:
- Around line 11-12: The .gitignore currently only excludes files named exactly
"**/.env", which still allows variants like ".env.local" or ".env.*" to be
committed; update the ignore patterns to cover env variants by replacing or
supplementing the existing "**/.env" entry with broader patterns such as
"**/.env*" and "**/.env.*" (and optionally specific ones like "**/.env.local" /
"**/.env.production") so all local env variants are ignored while keeping the
existing "**/magicblock-test-storage" entry intact.
In `@anchor-counter-privy/client/.env.local`:
- Around line 1-4: The committed .env.local contains concrete secrets/URLs
(REACT_APP_PROVIDER_ENDPOINT, REACT_APP_EPHEMERAL_PROVIDER_ENDPOINT,
REACT_APP_TEE_PROVIDER_ENDPOINT, REACT_APP_PRIVY_APP_ID); remove this file from
the commit and replace it with a non-committed local file and a sanitized
.env.example containing placeholder values (e.g.,
REACT_APP_PROVIDER_ENDPOINT=https://your-provider,
REACT_APP_PRIVY_APP_ID=YOUR_PRIVY_APP_ID) and ensure .env.local is listed in
.gitignore so local env files are not tracked; if any secret value (like the
PRIVY_APP_ID) has been leaked, rotate it in the provider and update callers to
use the new value from local envs.
In `@anchor-counter-privy/client/craco.config.js`:
- Around line 24-33: The current code overwrites webpackConfig.resolve.fallback,
dropping upstream fallbacks; instead merge into the existing object so you
preserve any prior entries — update the site where
webpackConfig.resolve.fallback is set to merge (e.g., Object.assign or spread)
the existing webpackConfig.resolve.fallback with your entries (http, https,
crypto, stream, buffer, zlib, url, vm) so you only add/override the needed
polyfills without replacing the whole fallback object.
In `@anchor-counter-privy/client/public/manifest.json`:
- Around line 2-3: Update the web app manifest entries "short_name" and "name"
in manifest.json to reflect the actual application identity instead of the
default Create React App labels: locate the keys "short_name" and "name" and
replace their placeholder values ("React App" / "Create React App Sample") with
the app's real short display name and full name (e.g., "Anchor Counter" and
"Anchor Counter — Privy Client") so PWAs and browser UI show the correct app
identity.
In `@anchor-counter-privy/client/README.md`:
- Line 21: Fix the README.md markdownlint issues by adding a language identifier
to every code fence (replace ``` with ```bash or appropriate language), ensure
there is a blank line above and below all headings and code fences (add or
remove blank lines so headings and fenced blocks are separated by a blank line),
remove any trailing whitespace and ensure the file ends with a single trailing
newline; apply these changes to every occurrence of the triple-backtick fences
and headings flagged in the review.
In `@anchor-counter-privy/client/src/App.tsx`:
- Around line 284-295: The increment path can dereference an undefined
transaction when counterProgramClient.current is null; in increaseCounterTx,
explicitly ensure counterProgramClient.current is defined and that the
transaction returned from
counterProgramClient.current.methods.increment().accounts({counter:
counterPda}).transaction() is non-null before calling transaction.add(...) and
submitTransaction; if either is missing, return early (or throw) to avoid
calling methods on undefined. Locate the check in increaseCounterTx and add a
guard around the call site using counterProgramClient.current and the resulting
transaction variable.
- Around line 231-235: The effect currently sets createWalletCalled.current =
true before awaiting createWallet(), which prevents retries if createWallet()
rejects; change the logic in the useEffect so the flag is set only after a
successful createWallet() (or set it back to false on catch) — update the block
that checks authenticated, privyReady, privyWallet and uses createWalletCalled
to call createWallet() and then on success set createWalletCalled.current =
true, and on failure reset it (or rethrow/log) so transient failures can be
retried; target the useEffect, createWalletCalled, and createWallet symbols when
applying the change.
- Around line 257-262: The early guard incorrectly requires
ephemeralConnection.current for all submissions; change the logic so that
ephemeralConnection.current is only required when ephemeral is true: replace the
unconditional if (!ephemeralConnection.current) return null with a conditional
check (if (ephemeral && !ephemeralConnection.current) return null) and keep
selection of targetConnection as const targetConnection = ephemeral ?
ephemeralConnection.current : provider.current.connection so non-ephemeral flows
use provider.current.connection even if ER init is pending; update any related
null-return behavior in the submitTransaction path accordingly.
- Around line 148-225: The init() path can reject or return early and never
clear the loading flag; update the init function and its callers so
setIsLoading(false) is always called on error or early exit: inside the
useEffect's init async function ensure every early return path (e.g., when
!privyWallet, when cancelled after failures, after catching TEE auth errors)
calls setIsLoading(false) before returning, and change the final
init().catch(console.error) to catch errors and call setIsLoading(false) (and
optionally setTransactionError) so load state is cleared on any rejection;
reference functions/variables to modify: useEffect -> init, setIsLoading,
setIsInitializingEr, cachedAuthToken.current, getAuthToken,
ephemeralConnection.current, and the init().catch(...) handler.
In `@anchor-counter-privy/client/src/components/Alert.tsx`:
- Around line 30-53: The Alert toast div in the Alert component lacks ARIA
semantics for screen readers; update the container (the div rendering {message}
in Alert.tsx) to include role="alert" and an appropriate aria-live attribute
(e.g., aria-live="assertive" for errors/important notices or "polite" for
non-urgent messages) and consider adding aria-atomic="true" so assistive tech
announces the full message; ensure these attributes are set conditionally based
on the component's type/importance if needed.
In `@anchor-counter-privy/client/src/components/FundWalletBanner.tsx`:
- Around line 13-17: handleCopy currently awaits
navigator.clipboard.writeText(address) with no error handling which can reject
(permissions/non-secure contexts) and leave copied state incorrect; wrap the
clipboard write in try/catch/finally (use navigator.clipboard?.writeText when
available), on success call setCopied(true) and start the reset timeout, on
failure log or surface an error (or fallback to programmatic copy via a
temporary textarea) and ensure setCopied(false) is called (or not set true) so
the UI never remains in a copied state; reference handleCopy, setCopied,
navigator.clipboard.writeText and address when making the change.
In `@anchor-counter-privy/client/src/components/PrivyConnectButton.tsx`:
- Around line 40-46: The copy control uses a plain <span> which is not
keyboard-accessible; change the interactive element for the address (the element
with className "privy-wallet-chip-addr" and the click handler handleCopy) so it
is keyboard operable: replace or convert the span into a semantic button (or add
role="button", tabIndex={0}, aria-label describing the action, and an onKeyDown
handler that calls handleCopy for Enter/Space) and preserve the title and
copied/short text behavior; ensure focus styles remain visible for keyboard
users.
- Around line 31-35: Wrap the clipboard write in handleCopy with a try/catch so
navigator.clipboard.writeText(address) failures are caught and setCopied(true)
is only called on success (handle or log the error in the catch and don’t call
setCopied); keep the setTimeout to clear copied as-is. Also make the clickable
address element keyboard-accessible by replacing the span with a semantic
<button> or, if you must keep a non-button element, add role="button",
tabIndex={0}, and an onKeyDown handler that triggers the same copy behavior on
Enter and Space; ensure you reference the same handleCopy and setCopied logic so
keyboard and pointer interactions behave identically.
In `@anchor-counter-privy/client/src/components/Square.tsx`:
- Around line 27-32: The interactive element uses a motion.div (in Square.tsx)
with onClick={handleClick}, which is not keyboard-accessible; change it to a
semantic button (use motion.button or render a button) so Enter/Space and
accessibility semantics work by default, keep the same className={classes} and
motion props (initial/animate), ensure you preserve handleClick as the onClick
handler, and if necessary reset default button styles in CSS so visuals don't
change.
In `@anchor-counter-privy/client/src/index.css`:
- Around line 423-429: The CSS uses descendant selectors by accident: change
".join-game input[type=\"text\"] ::placeholder" to ".join-game
input[type=\"text\"]::placeholder" and ".join-game input[type=\"text\"] :focus"
to ".join-game input[type=\"text\"]:focus" so the placeholder color and focus
styles apply to the input itself; also ensure the focus rule has a closing brace
and any desired vendor-prefixed placeholder selectors are added if needed.
- Around line 1-11: Remove the duplicate Google Fonts import (`@import` ...) from
client/src/index.css and update the body font declaration to satisfy the linter:
delete the `@import` rule and change the body font-family from 'Roboto',
sans-serif to an unquoted Roboto token (font-family: Roboto, sans-serif) so it
no longer triggers import-notation and font-family-name-quotes; target the
`@import` rule and the body selector in index.css when making the change.
In `@anchor-counter-privy/package.json`:
- Around line 3-4: Update the package.json lint scripts ("lint" and "lint:fix")
to include TSX files by adding .tsx to the globs; specifically, adjust the
patterns used in both "prettier */*.js \"*/**/*{.js,.ts}\" -w" and "prettier
*/*.js \"*/**/*{.js,.ts}\" --check" so they also match .tsx (e.g., add */*.tsx
and change "{.js,.ts}" to "{.js,.ts,.tsx}") so App.tsx and other component .tsx
files are included in yarn lint and lint:fix.
In `@anchor-counter-privy/programs/private-counter/src/lib.rs`:
- Around line 33-38: The increment function resets counter.count to 0 when it
exceeds 1000, but the other increment paths (increment_and_commit and
increment_and_undelegate) lack this rollover, causing divergence; update those
functions (increment_and_commit and increment_and_undelegate) to apply the same
rollover logic after incrementing counter.count (e.g., if counter.count > 1000 {
counter.count = 0 }) or extract the increment+rollover into a shared helper used
by all three functions to ensure consistent behavior for the counter account.
- Around line 24-27: The initialize function is acting as a reset endpoint
because it unconditionally sets ctx.accounts.counter.count = 0; change it to a
one-time init by using an account constraint that performs init (not
init_if_needed) or add an explicit one-time guard before mutating the account:
ensure the PDA is created only once (use the Anchor init attribute on the
Counter account in the Initialize context or check a boolean/owner flag on
Counter and return an error if already initialized) and only then set
counter.count; update the Initialize instruction handler and the associated
context/account definition to use init (or enforce the guard) instead of
allowing repeated zeroing.
In `@anchor-counter-privy/programs/public-counter/src/lib.rs`:
- Around line 16-19: The initialize handler currently unconditionally resets the
PDA (function initialize, variable counter in Context<Initialize>, lines setting
counter.count and msg!), allowing anyone to wipe state; change the account
constraint in the Initialize account struct from init_if_needed to init so the
PDA must be created only once, or alternatively add an explicit guard at the top
of initialize (e.g., check the account's initialized state or that counter.count
is uninitialized) and return an error if the account already exists to prevent
overwriting existing counter state; update the account constraint or guard
consistently wherever init_if_needed is used (the same pattern around the code
that references init_if_needed) to ensure existing PDAs are rejected.
- Around line 24-29: The wrap-to-zero logic in pub fn increment (check
counter.count > 1000 then set to 0) must be applied identically to the combined
flows that also commit or undelegate; update the increment logic in the combined
handlers (the functions that operate on ctx.accounts.counter when performing
commit/undelegate in addition to increment) to use the same check and reset on
counter.count (use the same mutable reference pattern let counter = &mut
ctx.accounts.counter; counter.count += 1; if counter.count > 1000 {
counter.count = 0; }) so the public API semantics are consistent across
increment, commit+increment, and undelegate+increment flows.
In `@anchor-counter-privy/README.md`:
- Around line 39-46: The README currently uses an absolute/mistyped cleanup
command "rm -rf /target/deploy/*.keypair" which won't remove Anchor's generated
deploy keypairs; update that line to the correct relative path and glob for
Anchor keypair filenames by replacing the command with one that targets
"target/deploy/*-keypair.json" (or "target/deploy/*.json") so the generated
<program>-keypair.json files are actually removed before running "anchor test".
In `@anchor-counter-privy/tests/private-counter.ts`:
- Around line 56-75: The provider is being authenticated against
ephemeralRpcEndpoint (via getAuthToken) but then reconnected using the
hard-coded teeUrl/teeWsUrl; update the providerEphemeralRollup construction to
use the resolved endpoints (ephemeralRpcEndpoint and the corresponding
ephemeralWsEndpoint variable) instead of teeUrl/teeWsUrl, ensuring the
connection URL and wsEndpoint include the ?token=${authToken.token} query
string; locate getAuthToken and providerEphemeralRollup in the diff and replace
teeUrl/teeWsUrl with the ephemeral* endpoint variables so the authenticated
token and connection target match.
- Around line 148-149: The test currently uses fixed sleeps (await new
Promise((resolve) => setTimeout(resolve, 3000)) and a similar 5000ms sleep) to
wait for delegation/undelegation to settle; replace those with a polling helper
(e.g., waitForCondition) that repeatedly queries the expected state from the ER
(use the same query used elsewhere in the test to verify delegation status)
until a predicate returns true or a timeout elapses, polling at a small interval
(e.g., 250–1000ms) and throwing on timeout; update the two occurrences (the 3s
and 5s sleeps) to call this helper with an appropriate timeout (e.g., 30s) and
interval instead of sleeping blindly.
- Around line 133-139: Replace the hard-coded validator PublicKey in the
.accounts({ validator: new web3.PublicKey("MTEW...") }) call with a resolved
value: read TEE_VALIDATOR_PUBKEY env var and if not present, derive the
validator identity from the configured TEE_PROVIDER_ENDPOINT
(TEE_PROVIDER_ENDPOINT) by querying that endpoint or a helper function (e.g.,
getValidatorPubkeyFromEndpoint) at test setup; assign the resulting PublicKey to
a local variable (validatorPubkey) and use validator: validatorPubkey in the
.accounts call, and throw a clear error if neither env var nor resolved value is
available so tests fail fast.
In `@anchor-counter-privy/tests/public-counter.ts`:
- Line 9: The test suite is using describe.only in tests/public-counter.ts which
prevents other tests from running; remove the .only (change
describe.only("public-counter", ...) to describe("public-counter", ...)) and
scan for any other .only or it.only usages in the test files (e.g.,
advanced-magic.ts, private-counter.ts) to ensure the full test suite runs in CI.
- Around line 129-130: Replace the fixed 3s sleep after logging txHash with an
explicit delegation-ready poll: after printing `${duration}ms (Base Layer)
Delegate txHash: ${txHash}` repeatedly query the network/client for delegation
confirmation (for example use provider.waitForTransaction(txHash) or a dedicated
checkDelegationStatus/getDelegation call) with a short backoff and an overall
timeout, and only proceed when the delegation is confirmed or fail the test on
timeout; update the code in public-counter.ts to remove the await new
Promise(...) sleep and implement this polling/timeout logic referencing txHash.
- Around line 99-115: The code currently hard-codes validator PublicKeys into
remainingAccounts causing delegation to a single remote validator; replace that
logic in the remainingAccounts assignment (which references
providerEphemeralRollup and rpcEndpoint) with a runtime lookup: call
providerEphemeralRollup.connection.getClusterNodes() or getVoteAccounts() to
select a valid validator node pubkey, construct a web3.PublicKey from that
result, and use it in the account entry; include a safe fallback (e.g., the
previous fallback keys) if the RPC call fails or the list is empty, and preserve
isSigner/isWritable flags as before.
🪄 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: f91c1daf-1dde-40b3-bf7f-1418e76473d9
⛔ Files ignored due to path filters (7)
anchor-counter-privy/Cargo.lockis excluded by!**/*.lockanchor-counter-privy/client/bun.lockis excluded by!**/*.lockanchor-counter-privy/client/package-lock.jsonis excluded by!**/package-lock.jsonanchor-counter-privy/client/public/favicon.icois excluded by!**/*.icoanchor-counter-privy/client/public/magicblock_white.pngis excluded by!**/*.pnganchor-counter-privy/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lockanchor-counter-privy/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
anchor-counter-privy/.claude/settings.jsonanchor-counter-privy/.env.exampleanchor-counter-privy/.gitignoreanchor-counter-privy/Anchor.tomlanchor-counter-privy/Cargo.tomlanchor-counter-privy/README.mdanchor-counter-privy/client/.env.exampleanchor-counter-privy/client/.env.localanchor-counter-privy/client/LICENSE.mdanchor-counter-privy/client/README.mdanchor-counter-privy/client/craco.config.jsanchor-counter-privy/client/package.jsonanchor-counter-privy/client/public/index.htmlanchor-counter-privy/client/public/manifest.jsonanchor-counter-privy/client/src/App.tsxanchor-counter-privy/client/src/components/Active.scssanchor-counter-privy/client/src/components/Active.tsxanchor-counter-privy/client/src/components/Alert.tsxanchor-counter-privy/client/src/components/Button.tsxanchor-counter-privy/client/src/components/FundWalletBanner.tsxanchor-counter-privy/client/src/components/PrivyConnectButton.tsxanchor-counter-privy/client/src/components/Square.scssanchor-counter-privy/client/src/components/Square.tsxanchor-counter-privy/client/src/components/Wallet.tsxanchor-counter-privy/client/src/index.cssanchor-counter-privy/client/src/index.tsxanchor-counter-privy/client/tsconfig.jsonanchor-counter-privy/package.jsonanchor-counter-privy/programs/private-counter/Cargo.tomlanchor-counter-privy/programs/private-counter/src/lib.rsanchor-counter-privy/programs/public-counter/Cargo.tomlanchor-counter-privy/programs/public-counter/src/lib.rsanchor-counter-privy/tests/advanced-magic.tsanchor-counter-privy/tests/private-counter.tsanchor-counter-privy/tests/public-counter.tsanchor-counter-privy/tsconfig.json
Comment on lines
+4
to
+10
| "Bash(yarn add *)", | ||
| "Bash(node -e \"const p = require\\('@privy-io/react-auth'\\); console.log\\(Object.keys\\(p\\).filter\\(k => k.toLowerCase\\(\\).includes\\('sol'\\) || k.toLowerCase\\(\\).includes\\('wallet'\\) || k.toLowerCase\\(\\).includes\\('privy'\\)\\)\\)\")", | ||
| "Bash(python3 -c \"import sys,json; p=json.load\\(sys.stdin\\); print\\(list\\(p.get\\('exports',{}\\).keys\\(\\)\\)[:20]\\)\")", | ||
| "Bash(node -e \"const p = require\\('@privy-io/react-auth/solana'\\); console.log\\(Object.keys\\(p\\)\\)\")", | ||
| "Bash(python3 -c \"import sys,json; p=json.load\\(sys.stdin\\); exports=p.get\\('exports',{}\\); [print\\(k,'->',v\\) for k,v in exports.items\\(\\) if 'sol' in k.lower\\(\\)]\")", | ||
| "Bash(npm list *)", | ||
| "Bash(npx cross-env *)" |
Contributor
There was a problem hiding this comment.
Narrow the Claude Bash allowlist.
These wildcard entries grant effectively arbitrary command execution (yarn add * can run package install scripts, and npx cross-env * can prefix almost any command). That is much broader than the read-only inspection use case described by the rest of this file and makes the checked-in agent config unsafe to reuse as-is.
🤖 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 `@anchor-counter-privy/.claude/settings.json` around lines 4 - 10, The
allowlist contains overly-broad wildcard Bash entries like "Bash(yarn add *)",
"Bash(npx cross-env *)" and similar patterns that permit arbitrary command
execution; replace those wildcard entries with a minimal, explicit set of safe
inspection commands or remove them entirely (for example, keep the specific
node/python inspection lines such as the existing "Bash(node -e ...)" and
"Bash(python3 -c ...)" entries) and instead whitelist exact invocations or
read-only flags (no "*" wildcards) for symbols like "Bash(yarn add *)",
"Bash(npm list *)", and "Bash(npx cross-env *)" so the agent can only run
predefined, read-only commands.
Comment on lines
+11
to
+12
| **/.env | ||
| **/magicblock-test-storage |
Contributor
There was a problem hiding this comment.
Ignore local env variants too.
Line 11 only ignores files named exactly .env, so client/.env.local and other .env.*.local files in this template can still be committed. That is a real leak path for local Privy/app endpoint configuration.
Suggested fix
**/.env
+**/.env.local
+**/.env.*.local
+!**/.env.example📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **/.env | |
| **/magicblock-test-storage | |
| **/.env | |
| **/.env.local | |
| **/.env.*.local | |
| !**/.env.example | |
| **/magicblock-test-storage |
🤖 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 `@anchor-counter-privy/.gitignore` around lines 11 - 12, The .gitignore
currently only excludes files named exactly "**/.env", which still allows
variants like ".env.local" or ".env.*" to be committed; update the ignore
patterns to cover env variants by replacing or supplementing the existing
"**/.env" entry with broader patterns such as "**/.env*" and "**/.env.*" (and
optionally specific ones like "**/.env.local" / "**/.env.production") so all
local env variants are ignored while keeping the existing
"**/magicblock-test-storage" entry intact.
Comment on lines
+1
to
+4
| REACT_APP_PROVIDER_ENDPOINT=https://api.devnet.solana.com | ||
| REACT_APP_EPHEMERAL_PROVIDER_ENDPOINT=https://devnet.magicblock.app | ||
| REACT_APP_TEE_PROVIDER_ENDPOINT=https://devnet-tee.magicblock.app | ||
| REACT_APP_PRIVY_APP_ID=clzpy568k01mla6yh4kjaoa4s |
Contributor
There was a problem hiding this comment.
Do not commit concrete values in .env.local.
Track .env.example only and keep .env.local untracked; committed local env files are a recurring secret/config leakage path across forks and contributors.
Suggested change
-REACT_APP_PROVIDER_ENDPOINT=https://api.devnet.solana.com
-REACT_APP_EPHEMERAL_PROVIDER_ENDPOINT=https://devnet.magicblock.app
-REACT_APP_TEE_PROVIDER_ENDPOINT=https://devnet-tee.magicblock.app
-REACT_APP_PRIVY_APP_ID=clzpy568k01mla6yh4kjaoa4s
+# Keep this file local-only; use `.env.example` for committed defaults/placeholders.🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [UnorderedKey] The REACT_APP_EPHEMERAL_PROVIDER_ENDPOINT key should go before the REACT_APP_PROVIDER_ENDPOINT key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The REACT_APP_PRIVY_APP_ID key should go before the REACT_APP_PROVIDER_ENDPOINT key
(UnorderedKey)
🤖 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 `@anchor-counter-privy/client/.env.local` around lines 1 - 4, The committed
.env.local contains concrete secrets/URLs (REACT_APP_PROVIDER_ENDPOINT,
REACT_APP_EPHEMERAL_PROVIDER_ENDPOINT, REACT_APP_TEE_PROVIDER_ENDPOINT,
REACT_APP_PRIVY_APP_ID); remove this file from the commit and replace it with a
non-committed local file and a sanitized .env.example containing placeholder
values (e.g., REACT_APP_PROVIDER_ENDPOINT=https://your-provider,
REACT_APP_PRIVY_APP_ID=YOUR_PRIVY_APP_ID) and ensure .env.local is listed in
.gitignore so local env files are not tracked; if any secret value (like the
PRIVY_APP_ID) has been leaked, rotate it in the provider and update callers to
use the new value from local envs.
Comment on lines
+24
to
+33
| webpackConfig.resolve.fallback = { | ||
| http: require.resolve('stream-http'), | ||
| https: require.resolve('https-browserify'), | ||
| crypto: require.resolve('crypto-browserify'), | ||
| stream: require.resolve('stream-browserify'), | ||
| buffer: require.resolve('buffer-browserify'), | ||
| zlib: false, | ||
| url: false, | ||
| vm: false, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Merge resolve.fallback instead of replacing it.
This assignment drops any fallback entries already present on the generated CRA config. Merging here is safer and avoids breaking other polyfills if the upstream config changes.
Suggested fix
- webpackConfig.resolve.fallback = {
+ webpackConfig.resolve = webpackConfig.resolve || {};
+ webpackConfig.resolve.fallback = {
+ ...(webpackConfig.resolve.fallback || {}),
http: require.resolve('stream-http'),
https: require.resolve('https-browserify'),
crypto: require.resolve('crypto-browserify'),
stream: require.resolve('stream-browserify'),
buffer: require.resolve('buffer-browserify'),
zlib: false,
url: false,
vm: false,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| webpackConfig.resolve.fallback = { | |
| http: require.resolve('stream-http'), | |
| https: require.resolve('https-browserify'), | |
| crypto: require.resolve('crypto-browserify'), | |
| stream: require.resolve('stream-browserify'), | |
| buffer: require.resolve('buffer-browserify'), | |
| zlib: false, | |
| url: false, | |
| vm: false, | |
| }; | |
| webpackConfig.resolve = webpackConfig.resolve || {}; | |
| webpackConfig.resolve.fallback = { | |
| ...(webpackConfig.resolve.fallback || {}), | |
| http: require.resolve('stream-http'), | |
| https: require.resolve('https-browserify'), | |
| crypto: require.resolve('crypto-browserify'), | |
| stream: require.resolve('stream-browserify'), | |
| buffer: require.resolve('buffer-browserify'), | |
| zlib: false, | |
| url: false, | |
| vm: false, | |
| }; |
🤖 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 `@anchor-counter-privy/client/craco.config.js` around lines 24 - 33, The
current code overwrites webpackConfig.resolve.fallback, dropping upstream
fallbacks; instead merge into the existing object so you preserve any prior
entries — update the site where webpackConfig.resolve.fallback is set to merge
(e.g., Object.assign or spread) the existing webpackConfig.resolve.fallback with
your entries (http, https, crypto, stream, buffer, zlib, url, vm) so you only
add/override the needed polyfills without replacing the whole fallback object.
Comment on lines
+2
to
+3
| "short_name": "React App", | ||
| "name": "Create React App Sample", |
Contributor
There was a problem hiding this comment.
Replace the default CRA manifest labels.
short_name and name still describe a generic Create React App, so installed PWAs and browser UI will show the wrong app identity.
Suggested fix
- "short_name": "React App",
- "name": "Create React App Sample",
+ "short_name": "Ephemeral Counter",
+ "name": "Ephemeral Counter",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "short_name": "React App", | |
| "name": "Create React App Sample", | |
| "short_name": "Ephemeral Counter", | |
| "name": "Ephemeral Counter", |
🤖 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 `@anchor-counter-privy/client/public/manifest.json` around lines 2 - 3, Update
the web app manifest entries "short_name" and "name" in manifest.json to reflect
the actual application identity instead of the default Create React App labels:
locate the keys "short_name" and "name" and replace their placeholder values
("React App" / "Create React App Sample") with the app's real short display name
and full name (e.g., "Anchor Counter" and "Anchor Counter — Privy Client") so
PWAs and browser UI show the correct app identity.
Comment on lines
+133
to
+139
| .accounts({ | ||
| payer: provider.wallet.publicKey, | ||
| // Pin to the TEE validator identity | ||
| validator: new web3.PublicKey( | ||
| "MTEWGuqxUpYZGFJQcp8tLN7x5v9BSeoFHYWQQ3n3xzo", | ||
| ), | ||
| }) |
Contributor
There was a problem hiding this comment.
Don't hard-code the TEE validator identity here.
Lines 136-138 always delegate to the devnet validator, even when TEE_PROVIDER_ENDPOINT points somewhere else. That makes the suite fail against any other TEE environment or after a validator rotation. Resolve the validator from the configured endpoint or make it an explicit env var.
🤖 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 `@anchor-counter-privy/tests/private-counter.ts` around lines 133 - 139,
Replace the hard-coded validator PublicKey in the .accounts({ validator: new
web3.PublicKey("MTEW...") }) call with a resolved value: read
TEE_VALIDATOR_PUBKEY env var and if not present, derive the validator identity
from the configured TEE_PROVIDER_ENDPOINT (TEE_PROVIDER_ENDPOINT) by querying
that endpoint or a helper function (e.g., getValidatorPubkeyFromEndpoint) at
test setup; assign the resulting PublicKey to a local variable (validatorPubkey)
and use validator: validatorPubkey in the .accounts call, and throw a clear
error if neither env var nor resolved value is available so tests fail fast.
Comment on lines
+148
to
+149
| // Wait for delegation to propagate to the ER | ||
| await new Promise((resolve) => setTimeout(resolve, 3000)); |
Contributor
There was a problem hiding this comment.
Replace the fixed sleeps with readiness/commitment polling.
Lines 149 and 233 assume delegation and undelegation settle within 3s/5s. On devnet/TEE infrastructure that is nondeterministic, so this suite will fail intermittently under load. Poll for the expected state with a timeout instead of sleeping blindly.
Also applies to: 232-233
🤖 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 `@anchor-counter-privy/tests/private-counter.ts` around lines 148 - 149, The
test currently uses fixed sleeps (await new Promise((resolve) =>
setTimeout(resolve, 3000)) and a similar 5000ms sleep) to wait for
delegation/undelegation to settle; replace those with a polling helper (e.g.,
waitForCondition) that repeatedly queries the expected state from the ER (use
the same query used elsewhere in the test to verify delegation status) until a
predicate returns true or a timeout elapses, polling at a small interval (e.g.,
250–1000ms) and throwing on timeout; update the two occurrences (the 3s and 5s
sleeps) to call this helper with an appropriate timeout (e.g., 30s) and interval
instead of sleeping blindly.
|
|
||
| const COUNTER_SEED = "counter"; | ||
|
|
||
| describe.only("public-counter", () => { |
Contributor
There was a problem hiding this comment.
Remove describe.only before merge.
Line 9 will skip every other test suite in this package, so CI can go green while advanced-magic.ts and private-counter.ts never run.
🤖 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 `@anchor-counter-privy/tests/public-counter.ts` at line 9, The test suite is
using describe.only in tests/public-counter.ts which prevents other tests from
running; remove the .only (change describe.only("public-counter", ...) to
describe("public-counter", ...)) and scan for any other .only or it.only usages
in the test files (e.g., advanced-magic.ts, private-counter.ts) to ensure the
full test suite runs in CI.
Comment on lines
+99
to
+115
| const remainingAccounts = | ||
| providerEphemeralRollup.connection.rpcEndpoint.includes("localhost") || | ||
| providerEphemeralRollup.connection.rpcEndpoint.includes("127.0.0.1") | ||
| ? [ | ||
| { | ||
| pubkey: new web3.PublicKey("mAGicPQYBMvcYveUZA5F5UNNwyHvfYh5xkLS2Fr1mev"), | ||
| isSigner: false, | ||
| isWritable: false, | ||
| }, | ||
| ] | ||
| : [ | ||
| { | ||
| pubkey: new web3.PublicKey("MAS1Dt9qreoRMQ14YQuhg8UTZMMzDdKhmkZMECCzk57"), | ||
| isSigner: false, | ||
| isWritable: false, | ||
| }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
Don't couple delegation to a single hard-coded remote validator.
Lines 109-114 always target one devnet validator whenever the endpoint is not local. If EPHEMERAL_PROVIDER_ENDPOINT is changed, or that validator rotates, the counter is delegated to the wrong place and the ER transactions that follow will fail.
🤖 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 `@anchor-counter-privy/tests/public-counter.ts` around lines 99 - 115, The code
currently hard-codes validator PublicKeys into remainingAccounts causing
delegation to a single remote validator; replace that logic in the
remainingAccounts assignment (which references providerEphemeralRollup and
rpcEndpoint) with a runtime lookup: call
providerEphemeralRollup.connection.getClusterNodes() or getVoteAccounts() to
select a valid validator node pubkey, construct a web3.PublicKey from that
result, and use it in the account entry; include a safe fallback (e.g., the
previous fallback keys) if the RPC call fails or the list is empty, and preserve
isSigner/isWritable flags as before.
Comment on lines
+129
to
+130
| console.log(`${duration}ms (Base Layer) Delegate txHash: ${txHash}`); | ||
| await new Promise((resolve) => setTimeout(resolve, 3000)); |
Contributor
There was a problem hiding this comment.
Use an explicit delegation-ready check instead of sleeping 3 seconds.
Line 130 assumes delegation propagation always finishes within 3 seconds. On shared devnet infrastructure that's not stable, so this test will intermittently fail even when the code is correct.
🤖 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 `@anchor-counter-privy/tests/public-counter.ts` around lines 129 - 130, Replace
the fixed 3s sleep after logging txHash with an explicit delegation-ready poll:
after printing `${duration}ms (Base Layer) Delegate txHash: ${txHash}`
repeatedly query the network/client for delegation confirmation (for example use
provider.waitForTransaction(txHash) or a dedicated
checkDelegationStatus/getDelegation call) with a short backoff and an overall
timeout, and only proceed when the delegation is confirmed or fail the test on
timeout; update the code in public-counter.ts to remove the await new
Promise(...) sleep and implement this polling/timeout logic referencing txHash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a new developer template (anchor-counter-privy) that shows how to use Magicblock's Ephemeral Rollups with Privy for wallet connections.
Previously, we were using temporary keypair files for wallet simulation, which isn't practical for real-world apps. This template replaces that with Privy's passwordless auth and embedded Solana wallets. It supports both public devnet ER and private/TEE rollup modes in a single React app.
Summary by CodeRabbit
New Features
Documentation
Tests