Scaffold program#7
Merged
Merged
Conversation
Update AGENTS.md to serve as a comprehensive project reference. Added sections for project overview, coding standards (Rust/TypeScript), development workflows, and architecture notes. Removed outdated references and clarified design decisions regarding zero-copy state, PDA derivation, and placeholder integrations.
There was a problem hiding this comment.
Pull request overview
Scaffolds the core Opal optimistic-oracle flow in the Anchor program (assertion → dispute → LLM resolution → vote escalation placeholder), adds zero-copy state accounts, and introduces Bun-based integration tests plus updated docs/config.
Changes:
- Replaces the template counter program with Opal protocol accounts, constants, errors, utilities, and instruction handlers for the assertion/dispute lifecycle.
- Adds a comprehensive localnet integration test suite (
tests/opal.test.ts) and a devnet smoke test. - Updates repo configuration/docs (Anchor test script, TS config, package deps/scripts, README + design docs).
Reviewed changes
Copilot reviewed 45 out of 50 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds Bun types and updates TS path alias configuration. |
| tests/opal.test.ts | Replaces counter tests with full localnet integration tests for the Opal flow and error cases. |
| tests/opal.devnet.test.ts | Adds a basic devnet smoke test for program deployment + config fetch. |
| programs/opal/src/utils.rs | Adds helpers for outcome mapping, bps math, and sentinel checks. |
| programs/opal/src/state/vote_resolution_round.rs | Introduces zero-copy vote round state (MagicBlock placeholder fields + vote aggregates). |
| programs/opal/src/state/vote_dispute.rs | Introduces zero-copy vote dispute account state. |
| programs/opal/src/state/protocol_config.rs | Introduces zero-copy singleton protocol config state. |
| programs/opal/src/state/mod.rs | Switches state module exports from template counter to Opal accounts. |
| programs/opal/src/state/llm_resolution_round.rs | Introduces zero-copy LLM resolution round state (Switchboard placeholder fields). |
| programs/opal/src/state/llm_dispute.rs | Introduces zero-copy LLM dispute account state. |
| programs/opal/src/state/counter.rs | Removes template counter account. |
| programs/opal/src/state/assertion.rs | Introduces zero-copy assertion account state with string buffers and pointers to disputes/rounds. |
| programs/opal/src/lib.rs | Replaces template entrypoints with Opal instruction entrypoints and re-exports. |
| programs/opal/src/instructions/submit_mock_llm_resolution.rs | Adds mock LLM resolution instruction (local testing) that updates LLM round + assertion. |
| programs/opal/src/instructions/open_vote.rs | Adds permissionless “open vote” transition (PendingVote → Voting) with placeholder delegation fields. |
| programs/opal/src/instructions/mod.rs | Updates instruction module exports to Opal instruction set. |
| programs/opal/src/instructions/initialize_protocol_config.rs | Adds protocol config initialization with invariants validation. |
| programs/opal/src/instructions/initialize.rs | Removes template initialize instruction. |
| programs/opal/src/instructions/increment.rs | Removes template increment instruction. |
| programs/opal/src/instructions/finalize_vote_resolution_placeholder.rs | Adds placeholder vote finalization + payout logic (no real voting yet). |
| programs/opal/src/instructions/finalize_undisputed.rs | Adds optimistic finalization path when undisputed after liveness window. |
| programs/opal/src/instructions/finalize_llm_resolution.rs | Adds LLM finalization path + payout logic after challenge window closes. |
| programs/opal/src/instructions/dispute_assertion.rs | Adds first-dispute instruction creating LLM dispute + LLM round and bonding. |
| programs/opal/src/instructions/decrement.rs | Removes template decrement instruction. |
| programs/opal/src/instructions/create_assertion.rs | Adds assertion creation instruction, bonding into a PDA-owned token vault. |
| programs/opal/src/instructions/challenge_llm_resolution.rs | Adds LLM-challenge instruction that escalates into vote dispute + vote round. |
| programs/opal/src/errors.rs | Replaces template errors with OpalError variants. |
| programs/opal/src/constants.rs | Adds PDA seeds, state constants, outcome constants, sentinel values. |
| programs/opal/Cargo.toml | Adds anchor-spl + MagicBlock SDK deps and adjusts features/lints. |
| package.json | Updates formatting scripts, adds Anchor/web3/spl-token deps, adjusts devnet test script. |
| docs/tokenomics.md | Formatting updates to parameter reference table. |
| docs/resolution.md | Formatting updates to outcome code mapping table. |
| docs/glossary.md | Formatting updates to quick reference table. |
| codama.json | Formatting-only update. |
| README.md | Updates protocol description, build/test instructions, and clarifies placeholders. |
| Anchor.toml | Points Anchor test script to bun test runner entrypoint. |
| AGENTS.md | Expands project overview, conventions, workflows, and architecture notes. |
| .gitignore | Ignores .env, .anchor, and generated web/client. |
| .agents/skills/surfpool/templates/test-setup.ts | Adds Surfpool test setup template (cheatcodes + helpers). |
| .agents/skills/surfpool/templates/Surfpool.toml | Adds Surfpool configuration template. |
| .agents/skills/surfpool/resources/github-repos.md | Adds Surfpool repository reference documentation. |
| .agents/skills/surfpool/resources/cli-reference.md | Adds Surfpool CLI reference documentation. |
| .agents/skills/surfpool/resources/cheatcodes.md | Adds Surfpool cheatcodes reference documentation. |
| .agents/skills/surfpool/examples/cheatcodes/state-manipulation.ts | Adds example code for Surfpool cheatcodes usage. |
| .agents/skills/surfpool/examples/basic/getting-started.ts | Adds getting-started example for Surfpool usage. |
| .agents/skills/surfpool/docs/troubleshooting.md | Adds Surfpool troubleshooting documentation. |
| .agents/skills/surfpool/SKILL.md | Adds Surfpool “skill” documentation bundle. |
| .agents/skills/caveman/SKILL.md | Adds “caveman mode” skill documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThis PR transforms a template Anchor counter program into a complete implementation of Opal, an optimistic oracle protocol on Solana. It introduces core state accounts (assertion, disputes, resolution rounds), protocol instructions for assertion lifecycle management (creation through finalization), governance configuration, and a full TypeScript integration test suite exercising the assertion → dispute → resolution flows with mock LLM resolution and placeholder voting mechanics. ChangesOpal Protocol Implementation
Sequence DiagramsequenceDiagram
participant User as Asserter / Disputer
participant Program as Opal Program
participant BondVault as Bond Vault (Token Account)
participant TokenProgram as SPL Token Program
User->>Program: create_assertion(statement, auxiliary_hash, bond_amount)
Program->>TokenProgram: transfer bond_amount PUSD to BondVault
Program->>Program: Initialize AssertionAccount (state=ASSERTED)
Note over Program: Assertion.liveness_deadline = now + liveness_window
User->>Program: dispute_assertion()
Program->>TokenProgram: transfer llm_bond_amount PUSD to BondVault
Program->>Program: Initialize LlmDisputeAccount & LlmResolutionRound
Program->>Program: Assertion.state = PENDING_LLM
User->>Program: submit_mock_llm_resolution(outcome_code)
Program->>Program: Set LlmResolutionRound.outcome & .challenge_deadline
Program->>Program: Assertion.state = ASSERTED_LLM
Note over Program: outcome_code mapped to u8 (0=TRUE, 1=FALSE, 2=TOO_EARLY, 3=UNRESOLVABLE)
alt Challenge Deadline Not Passed
User->>Program: challenge_llm_resolution()
Program->>TokenProgram: transfer vote_bond_amount PUSD to BondVault
Program->>Program: Initialize VoteDisputeAccount & VoteResolutionRound
Program->>Program: Assertion.state = PENDING_VOTE
User->>Program: open_vote()
Program->>Program: Set VoteResolutionRound vote deadlines
Program->>Program: Assertion.state = VOTING
User->>Program: finalize_vote_resolution_placeholder(outcome_code)
Program->>TokenProgram: Distribute bonds: payouts to correct parties + protocol fee to treasury
Program->>Program: Set all disputes.settlement_resolution & Assertion.outcome
Program->>Program: Assertion.state = RESOLVED
else Challenge Deadline Passed
User->>Program: finalize_llm_resolution()
Program->>TokenProgram: Distribute bonds: payouts to winner + protocol fee to treasury
Program->>Program: Set llm_dispute.settlement_resolution & Assertion.outcome
Program->>Program: Assertion.state = RESOLVED
end
alt No Dispute Filed
User->>Program: finalize_undisputed()
Program->>TokenProgram: Return assertion bond to asserter (net of protocol fee)
Program->>TokenProgram: Transfer protocol fee to treasury
Program->>Program: Assertion.state = RESOLVED, outcome = TRUE
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces substantial new protocol logic across multiple instruction handlers with intricate payout calculations, zero-copy account layouts, and state-machine transitions. While individual files are often straightforward, the heterogeneity of instruction implementations (mock LLM resolution, placeholder vote settlement, multiple finalization paths with distinct payout logic), the density of bond/fee arithmetic with overflow checks, and the integration across 9+ new instruction modules demand careful verification of account constraints, state invariants, and token transfer correctness. The test suite is comprehensive but also requires review for coverage and assertion accuracy. Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (7)
.agents/skills/surfpool/resources/cli-reference.md (1)
103-109: 💤 Low valueAdd language specifier to fenced code block.
The code block showing version output is missing a language specifier.
📝 Suggested fix
**Output:** -``` +```text Surfpool v1.0.0 Solana Core: 1.18.x Feature Set: 123456789</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/surfpool/resources/cli-reference.md around lines 103 - 109,
The fenced code block under the "Output:" section that shows "Surfpool
v1.0.0 / Solana Core: 1.18.x / Feature Set: 123456789" lacks a language tag;
update that Markdown block to use a language specifier (e.g., "text") so the
block becomestext ...to ensure proper rendering and syntax highlighting
in the cli-reference.md output example.</details> </blockquote></details> <details> <summary>tests/opal.devnet.test.ts (1)</summary><blockquote> `41-44`: _💤 Low value_ **Empty catch block masks potential issues.** The `expect(true).toBe(true)` in the catch block silently passes regardless of failure reason. Consider logging or distinguishing expected "not initialized" errors from unexpected failures. <details> <summary>💡 Suggested improvement</summary> ```diff try { const config = await program.account.protocolConfig.fetch(configPda); expect(config).toBeDefined(); expect(config.assertionBondMinPusd.toNumber()).toBeGreaterThan(0); - } catch { + } catch (e) { // Config may not be initialized on devnet; that's okay for smoke test - expect(true).toBe(true); + console.log("Protocol config not initialized on devnet (expected):", e); } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/opal.devnet.test.ts` around lines 41 - 44, Replace the empty catch block in tests/opal.devnet.test.ts with an error-aware handler: catch (err) and distinguish expected "not initialized" errors from unexpected failures by either asserting the error message/type (e.g., check err.message contains "not initialized" or instanceof a known ConfigError) or logging and rethrowing/failing the test for unexpected errors; ensure the catch references the test's operation (the code inside the try) so expected devnet init errors are accepted but anything else calls fail() or console.error(err) and fails the test. ``` </details> </blockquote></details> <details> <summary>.agents/skills/surfpool/examples/basic/getting-started.ts (1)</summary><blockquote> `133-145`: _⚡ Quick win_ **Replace private `connection._rpcRequest()` usage with a public JSON-RPC helper.** Lines 133-145 depend on the undocumented, private `_rpcRequest` method and suppress typing with `@ts-ignore`. Although these custom RPC methods (`surfnet_getClock`, `surfnet_getSurfpoolVersion`) are specific to Surfpool and not exposed by web3.js, wrapping the calls in a helper function using `fetch` would make the code more maintainable and shield against potential breaking changes in web3.js. A similar pattern is also used in `state-manipulation.ts`. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/surfpool/examples/basic/getting-started.ts around lines 133 - 145, Replace direct use of the private connection._rpcRequest in getSurfnetClock and getSurfpoolVersion by extracting a public JSON-RPC helper (e.g., jsonRpcRequest or fetchJsonRpc) that performs a POST fetch to the provider URL and returns the parsed result; update getSurfnetClock(connection) and getSurfpoolVersion(connection) to call that helper with method "surfnet_getClock" and "surfnet_getSurfpoolVersion" respectively and remove the `@ts-ignore` lines, ensuring the helper accepts a provider or endpoint derived from Connection and returns result.result. ``` </details> </blockquote></details> <details> <summary>programs/opal/src/instructions/submit_mock_llm_resolution.rs (1)</summary><blockquote> `49-67`: _💤 Low value_ **Minor: Redundant `protocol_config` reload.** The handler loads `protocol_config` twice (lines 49 and 63). You could avoid the second load by extracting `llm_challenge_window_seconds` before the first `drop()`. This is a minor optimization and doesn't affect correctness. <details> <summary>♻️ Optional simplification</summary> ```diff let protocol_config = ctx.accounts.protocol_config.load()?; require!( ctx.accounts.authority.key() == protocol_config.authority, OpalError::Unauthorized ); - drop(protocol_config); - - let assertion = ctx.accounts.assertion.load()?; - require!( - assertion.state == ASSERTION_STATE_PENDING_LLM, - OpalError::InvalidState - ); - drop(assertion); - - let protocol_config = ctx.accounts.protocol_config.load()?; + let llm_challenge_window_seconds = protocol_config.llm_challenge_window_seconds; + drop(protocol_config); + + let assertion = ctx.accounts.assertion.load()?; + require!( + assertion.state == ASSERTION_STATE_PENDING_LLM, + OpalError::InvalidState + ); + drop(assertion); + let outcome = map_outcome_code(args.outcome_code)?; let now = Clock::get()?.unix_timestamp; - let challenge_deadline = checked_add_i64(now, protocol_config.llm_challenge_window_seconds)?; - drop(protocol_config); + let challenge_deadline = checked_add_i64(now, llm_challenge_window_seconds)?; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@programs/opal/src/instructions/submit_mock_llm_resolution.rs` around lines 49 - 67, The code reloads protocol_config twice; instead, read protocol_config once, store protocol_config.llm_challenge_window_seconds in a local variable before calling drop(protocol_config), then use that local variable when computing challenge_deadline (replace the second load and its checked_add_i64 call). Update references in this handler (around protocol_config.load(), drop(protocol_config), and where checked_add_i64 is called) to use the new local llm_challenge_window_seconds variable. ``` </details> </blockquote></details> <details> <summary>programs/opal/src/instructions/mod.rs (1)</summary><blockquote> `11-28`: _💤 Low value_ **Consider consolidating the `#[allow]` attributes.** Each re-export has its own `#[allow(ambiguous_glob_reexports)]`. You could apply it once at module level or group the re-exports under a single attribute block. ```diff +#[allow(ambiguous_glob_reexports)] +mod exports { + pub use super::challenge_llm_resolution::*; + pub use super::create_assertion::*; + // ... etc +} +pub use exports::*; ``` This is purely cosmetic and the current approach works correctly. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@programs/opal/src/instructions/mod.rs` around lines 11 - 28, Remove the repeated #[allow(ambiguous_glob_reexports)] on each pub use and apply a single #[allow(ambiguous_glob_reexports)] once for the module or a single attribute block that precedes the grouped re-exports (the block containing pub use challenge_llm_resolution::*, create_assertion::*, dispute_assertion::*, finalize_llm_resolution::*, finalize_undisputed::*, finalize_vote_resolution_placeholder::*, initialize_protocol_config::*, open_vote::*, submit_mock_llm_resolution::*); this keeps the same behavior but consolidates the attribute for clarity. ``` </details> </blockquote></details> <details> <summary>.agents/skills/surfpool/templates/test-setup.ts (2)</summary><blockquote> `67-74`: _💤 Low value_ **Internal RPC method access is fragile.** Using `this.connection._rpcRequest` relies on an internal/private API that may change across `@solana/web3.js` versions. Consider adding a version check or documenting this dependency explicitly. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/surfpool/templates/test-setup.ts around lines 67 - 74, The cheatcode method uses the internal API this.connection._rpcRequest which is fragile; update cheatcode to first assert and guard that this.connection._rpcRequest exists and is a function (throw a clear error if not), add a fallback path or call using the public connection.request or send method when available, and/or perform a runtime version check of `@solana/web3.js` (or document the required version) so the code doesn't silently break; reference the cheatcode function and this.connection._rpcRequest when making the change and ensure the thrown error clearly instructs to upgrade/downgrade or use the expected library version. ``` </details> --- `189-197`: _⚡ Quick win_ **Silent error swallowing may hide issues.** `getTokenBalance` catches all errors and returns `0`, which could mask real problems (e.g., network issues, wrong mint). Consider logging a warning or only catching specific "account not found" errors. ```diff async getTokenBalance(owner: PublicKey, mint: PublicKey): Promise<bigint> { const ata = await getAssociatedTokenAddress(mint, owner); try { const account = await this.connection.getTokenAccountBalance(ata); return BigInt(account.value.amount); - } catch { + } catch (e: any) { + // Only treat "account does not exist" as zero balance + if (e?.message?.includes("could not find account")) { + return BigInt(0); + } + throw e; } - return BigInt(0); - } } ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/surfpool/templates/test-setup.ts around lines 189 - 197, The getTokenBalance function currently swallows all errors and returns 0; update getTokenBalance to only treat a missing/nonexistent token account as a zero balance and surface or log other failures: call getAssociatedTokenAddress and connection.getTokenAccountBalance as before but catch only the "account not found" case (or check for a null/404 response) to return BigInt(0), while logging a warning via your logger or rethrowing the error for other exceptions so network/mint problems are not silently ignored. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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 @.agents/skills/surfpool/examples/basic/getting-started.ts:
- Around line 232-244: The module currently auto-runs main() on import which
causes import-time side effects and can terminate callers; wrap the call in a
module-entry guard so it only runs when executed directly (e.g. replace the bare
"main().catch(console.error);" with a conditional like "if (require.main ===
module) { main().catch(console.error); }" or the ES module equivalent) while
keeping the exported helpers (CONFIG, waitForSurfnet, requestAirdrop,
getBalance, sendSol, getSurfnetClock, getSurfpoolVersion) unchanged.In @.agents/skills/surfpool/examples/cheatcodes/state-manipulation.ts:
- Around line 363-382: The module currently calls main() unconditionally which
creates a side-effect despite exporting reusable helpers (callCheatcode,
setAccount, setTokenAccount, cloneProgram, resetAccount, timeTravel, pauseClock,
resumeClock, advanceClock, getClock, resetNetwork, getVersion,
profileTransaction, getProfileResults); change it so main() only runs when the
file is executed as an entrypoint by wrapping the invocation in an
import.meta.main check (if (import.meta.main) { ... }), and on error ensure the
process exits with a non-zero status (e.g., console.error(err) then
process.exit(1)) instead of just logging, leaving the exported helper functions
importable without side effects.In @.agents/skills/surfpool/resources/github-repos.md:
- Around line 28-40: Replace the insecure "curl -sL https://run.surfpool.run/ |
bash" recommendation with a safe alternative: remove that pipe-to-shell line and
instead link to the official installation documentation or provide a versioned
download with checksum/signature verification instructions; explicitly recommend
vetted methods such as the Homebrew entry and the "From source" steps already
present and show how to verify a release artifact (checksum/signature) for any
binary installer. Ensure the change targets the block that currently contains
the exact command string "curl -sL https://run.surfpool.run/ | bash" so the docs
no longer recommend piping remote scripts into bash.In @.agents/skills/surfpool/SKILL.md:
- Line 492: The fenced code block that shows the project tree (the block
starting with "surpool/") is missing a language tag and triggers MD040; update
the opening fence fromtotext (or another appropriate language
identifier) in SKILL.md so the block is explicitly tagged (the tree block
starting "surpool/" is the unique snippet to change).In
@AGENTS.md:
- Around line 87-103: The two unlabeled fenced code blocks need a language to
satisfy markdownlint: change the opening triple backticks for the block starting
with "programs/opal/src/" to "text" and do the same for the ASCII state diagram block beginning "Asserted → (disputed?) PendingLlm → AssertedLlm..." (and the identical occurrence referenced at lines 212-216) so both blocks are fenced astext rather than plain ```; update the three opening fences
accordingly without altering the block contents.- Around line 163-176: The doc currently lists
bun run gen:clientin the
primary build steps while acknowledging it fails due to a circular dependency in
the Codama visitor; either fix the generator or remove/relocate that command
from the default happy-path. Update the AGENTS.md section soanchor build
remains the required step (it producestarget/idl/opal.jsonand
target/types/opal.ts), and movebun run gen:clientinto an “Optional /
Experimental” or “Troubleshooting” subsection (or add a clear gated note)
referencing the failing Codama visitor/circular-dependency issue so contributors
aren’t instructed to run a known-broken command. Ensure the text still states
build order (anchor buildfirst) and where the generated client would land
(web/client/src/generated/) if the generator is fixed.- Around line 254-259: The test suite currently sets skipPreflight: true on all
.rpc() calls (38 instances in tests/opal.test.ts), which hides simulation
failures; update tests to remove skipPreflight: true from every .rpc() call
except the explicitly flaky paths (identify and keep it only on the small set of
calls that intermittently throw "Unknown action 'undefined'"), so: locate all
usages of .rpc(...) in tests/opal.test.ts and remove the skipPreflight option
where present, add skipPreflight: true only to the confirmed flaky call sites
(leave a single-line TODO referencing the Anchor bug/issue near each kept
instance), and run the test suite to ensure no other failures; use the .rpc(...)
call sites and the skipPreflight symbol to find/change the code.In
@docs/architecture.md:
- Around line 15-16: Update the docs to describe on-chain fixed-size byte arrays
as zero-padded (not null-terminated): replace references to "null-terminated
string" with "zero-padded fixed-size byte array" and mention the max sizes
(statement max 280 bytes, auxiliary_hash max 128 bytes); reference the
create_assertion implementation
(programs/opal/src/instructions/create_assertion.rs) which copies bytes and
zero-fills the remainder so integrators know they must trim trailing zero bytes
when decoding.- Around line 202-253: Assertions can get permanently stuck in PENDING_LLM,
PENDING_VOTE or VOTING leaving bonds in BondVault; add explicit recovery
transitions and caller-authorizations that time out those states and
refund/settle bonds. Specifically: add config timeouts (e.g.,
llm_response_timeout, vote_open_timeout, vote_finalization_timeout) and
implement new entrypoints finalize_stalled_llm, finalize_stalled_vote, and
finalize_stalled_voting (or extend finalize_undisputed) that any caller can
invoke after the configured deadline to set AssertionAccount.state to RESOLVED
(or CANCELLED), compute settlement_resolution on LlmDisputeAccount and
VoteDisputeAccount, set
LlmResolutionRound.final_outcome/VoteResolutionRound.final_outcome to a
default/timeout outcome, and move funds from BondVault back to participants per
existing settlement logic used by
finalize_llm_resolution/finalize_vote_resolution_placeholder; ensure these
functions reference existing symbols (AssertionAccount.state, liveness_deadline,
LlmResolutionRound, VoteResolutionRound, LlmDisputeAccount, VoteDisputeAccount,
BondVault, create_assertion, dispute_assertion, submit_mock_llm_resolution,
open_vote, finalize_vote_resolution_placeholder) and enforce the "after timeout"
checks and idempotency.In
@docs/tokenomics.md:
- Around line 36-48: The fenced formula blocks for llm_dispute_bond and
vote_dispute_bond are missing a language specifier (triggers markdownlint
MD040); update the two code fences that contain the formulas for
llm_dispute_bond and vote_dispute_bond to include a language (e.g., usetext) so the blocks read astext ... ``` for both occurrences.In
@programs/opal/src/instructions/finalize_llm_resolution.rs:
- Around line 150-170: The finalize reads protocol_config.protocol_fee_bps at
finalization which allows the fee to change between dispute creation and
resolution; to fix, add an applicable_fee_bps (or similar) field to
LlmDisputeAccount, populate that field when the dispute is created, and replace
uses of protocol_config.protocol_fee_bps in finalize_llm_resolution (the code
block computing fee, llm_payout, asserter_total) with the stored
LlmDisputeAccount.applicable_fee_bps; ensure the dispute-creation flow that
constructs LlmDisputeAccount sets the new field and adjust
deserialization/serialization types accordingly (or, if intentional, update
docs/comments near LlmDisputeAccount and finalize_llm_resolution to state fees
are taken at finalize time).In
@programs/opal/src/instructions/finalize_undisputed.rs:
- Around line 89-91: Replace the direct use of
assertion_bond.checked_sub(fee).ok_or(OpalError::MathOverflow)? with the shared
token-math helper from programs/opal/src/utils.rs (e.g., the checked_sub helper)
so overflow handling is consistent; call the utils::checked_sub(assertion_bond,
fee) (or the similarly named helper in utils.rs) and propagate its Result
instead of using raw .checked_sub, keeping the resulting variable name as
asserter_payout and preserving the OpalError::MathOverflow mapping if the helper
returns that error variant.- Line 88: The settlement currently uses protocol_config.protocol_fee_bps inside
finalize_undisputed via checked_bps(assertion_bond,
protocol_config.protocol_fee_bps) which allows governance changes to
retroactively alter payouts; instead read the frozen fee bps stored on the
assertion/dispute state (e.g., use the field on Assertion or Dispute like
protocol_fee_bps_snapshot or fee_bps_at_creation) and pass that snapshot into
checked_bps. Update the creation path that constructs the Assertion/Dispute to
capture protocol_config.protocol_fee_bps into that field, and change
finalize_undisputed to use that field (and leave checked_bps and
finalize_undisputed signatures unchanged).In
@programs/opal/src/instructions/finalize_vote_resolution_placeholder.rs:
- Around line 160-234: This code reuses raw checked_add/checked_sub math inline
(e.g., in computing required_vault, payout, bonus, treasury_fee and when adding
stage_winner_bonus to asserter_payout/llm_disputer_payout) instead of the shared
safe helpers; update finalize_vote_resolution_placeholder.rs to call the checked
u64 helpers in programs/opal/src/utils.rs (or add the missing u64 helper there)
for every arithmetic operation (required_vault, fee/payout calculations that
currently call .checked_add/.checked_sub, stage_winner bonus addition, and final
treasury_fee), replace the inline .checked_add/.checked_sub chains with the
corresponding utils::checked_add_u64/checked_sub_u64 (or similarly named
helpers) and propagate the existing OpalError::MathOverflow on failure to keep
semantics identical.- Around line 29-35: The authority signer is not being validated against the
stored protocol authority in ProtocolConfig, allowing any signer to settle
votes; fix by ensuring the provided authority equals
protocol_config.authority—either add an Anchor account constraint on the
protocol_config field like #[account(seeds = [PROTOCOL_CONFIG_SEED], bump,
has_one = authority)] for the protocol_config AccountLoader<'info,
ProtocolConfig> declaration, or perform an explicit runtime check in
finalize_vote_resolution_placeholder (e.g., let pc = protocol_config.load()?;
require_keys_eq!(pc.authority, authority.key(), SomeError)) before applying the
outcome_code.In
@programs/opal/src/instructions/initialize_protocol_config.rs:
- Around line 54-55: Uncomment and re-enable the authority check placeholder so
only the program deployer can initialize config: remove the comment markers
around require!(ctx.accounts.authority.key() == crate::ID,
OpalError::Unauthorized) in initialize_protocol_config (restore the require!
call), ensuring the check uses ctx.accounts.authority.key(), compares to
crate::ID and returns OpalError::Unauthorized on mismatch.In
@programs/opal/src/utils.rs:
- Line 1: Replace the magic literals
3and0in programs/opal/src/utils.rs
with named constants from programs/opal/src/constants.rs: define or use
descriptive constants (for example SENTINEL_DOMAIN or PDA_DOMAIN_SENTINEL for
3, and SENTINEL_NONE or PDA_STATE_EMPTY for0) in constants.rs, import them
alongside BPS_DENOMINATOR and OpalError, and update all comparisons/assignments
that use the literal3or0to use these constants (3→ SENTINEL_DOMAIN,
0→ SENTINEL_NONE) so PDA seeds/state/sentinel values come from the shared
constants file.In
@tests/opal.devnet.test.ts:
- Around line 16-22: Replace the hardcoded local wallet used in the test setup:
in the beforeAll block that creates the AnchorProvider with new
AnchorProvider(connection, Wallet.local(), {commitment: "confirmed"}), switch to
using AnchorProvider.env() so the provider reads credentials from the
environment (set ANCHOR_PROVIDER_URL to DEVNET_RPC and ANCHOR_WALLET to the
keypair path) and then pass that provider into new Program(idl as any,
provider); update references to Wallet.local() to AnchorProvider.env() so CI/CD
and devnet runs can locate the wallet via environment variables.In
@tests/opal.test.ts:
- Around line 592-632: Re-add the missing negative test that ensures
finalizeUndisputed() is rejected if called before the assertion's
challenge_deadline: create an assertion via program.methods.createAssertion (use
Keypair.generate() for assertion id and derivePDAs(id, program.programId) to get
assertion and bondVault), then immediately call
program.methods.finalizeUndisputed() with the same accounts (finalizer
provider.wallet.publicKey, protocolConfig proto.configPda, pusdMint token.mint,
assertion, bondVault, asserterPusd token.asserterAta, treasuryPusd
proto.treasuryAta, tokenProgram TOKEN_PROGRAM_ID) and assert the RPC call
rejects (rejects.toThrow()). Ensure the test name describes "finalize before
challenge deadline" and keep the same signers and commit/skipPreflight options
as the existing createAssertion flow.
Nitpick comments:
In @.agents/skills/surfpool/examples/basic/getting-started.ts:
- Around line 133-145: Replace direct use of the private connection._rpcRequest
in getSurfnetClock and getSurfpoolVersion by extracting a public JSON-RPC helper
(e.g., jsonRpcRequest or fetchJsonRpc) that performs a POST fetch to the
provider URL and returns the parsed result; update getSurfnetClock(connection)
and getSurfpoolVersion(connection) to call that helper with method
"surfnet_getClock" and "surfnet_getSurfpoolVersion" respectively and remove the
@ts-ignorelines, ensuring the helper accepts a provider or endpoint derived
from Connection and returns result.result.In @.agents/skills/surfpool/resources/cli-reference.md:
- Around line 103-109: The fenced code block under the "Output:" section
that shows "Surfpool v1.0.0 / Solana Core: 1.18.x / Feature Set: 123456789"
lacks a language tag; update that Markdown block to use a language specifier
(e.g., "text") so the block becomestext ...to ensure proper rendering
and syntax highlighting in the cli-reference.md output example.In @.agents/skills/surfpool/templates/test-setup.ts:
- Around line 67-74: The cheatcode method uses the internal API
this.connection._rpcRequest which is fragile; update cheatcode to first assert
and guard that this.connection._rpcRequest exists and is a function (throw a
clear error if not), add a fallback path or call using the public
connection.request or send method when available, and/or perform a runtime
version check of@solana/web3.js(or document the required version) so the code
doesn't silently break; reference the cheatcode function and
this.connection._rpcRequest when making the change and ensure the thrown error
clearly instructs to upgrade/downgrade or use the expected library version.- Around line 189-197: The getTokenBalance function currently swallows all
errors and returns 0; update getTokenBalance to only treat a missing/nonexistent
token account as a zero balance and surface or log other failures: call
getAssociatedTokenAddress and connection.getTokenAccountBalance as before but
catch only the "account not found" case (or check for a null/404 response) to
return BigInt(0), while logging a warning via your logger or rethrowing the
error for other exceptions so network/mint problems are not silently ignored.In
@programs/opal/src/instructions/mod.rs:
- Around line 11-28: Remove the repeated #[allow(ambiguous_glob_reexports)] on
each pub use and apply a single #[allow(ambiguous_glob_reexports)] once for the
module or a single attribute block that precedes the grouped re-exports (the
block containing pub use challenge_llm_resolution::, create_assertion::,
dispute_assertion::, finalize_llm_resolution::, finalize_undisputed::,
finalize_vote_resolution_placeholder::, initialize_protocol_config::,
open_vote::, submit_mock_llm_resolution::*); this keeps the same behavior but
consolidates the attribute for clarity.In
@programs/opal/src/instructions/submit_mock_llm_resolution.rs:
- Around line 49-67: The code reloads protocol_config twice; instead, read
protocol_config once, store protocol_config.llm_challenge_window_seconds in a
local variable before calling drop(protocol_config), then use that local
variable when computing challenge_deadline (replace the second load and its
checked_add_i64 call). Update references in this handler (around
protocol_config.load(), drop(protocol_config), and where checked_add_i64 is
called) to use the new local llm_challenge_window_seconds variable.In
@tests/opal.devnet.test.ts:
- Around line 41-44: Replace the empty catch block in tests/opal.devnet.test.ts
with an error-aware handler: catch (err) and distinguish expected "not
initialized" errors from unexpected failures by either asserting the error
message/type (e.g., check err.message contains "not initialized" or instanceof a
known ConfigError) or logging and rethrowing/failing the test for unexpected
errors; ensure the catch references the test's operation (the code inside the
try) so expected devnet init errors are accepted but anything else calls fail()
or console.error(err) and fails the test.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `4e66ec90-9029-43ec-bf95-0ff69f76335f` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 60bea0cfda22c2429e0877b76d380d93a8f0393e and 78ba254ff3de567683642e1adb93ea395ff4593c. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `Cargo.lock` is excluded by `!**/*.lock` * `bun.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (49)</summary> * `.agents/skills/caveman/SKILL.md` * `.agents/skills/surfpool/SKILL.md` * `.agents/skills/surfpool/docs/troubleshooting.md` * `.agents/skills/surfpool/examples/basic/getting-started.ts` * `.agents/skills/surfpool/examples/cheatcodes/state-manipulation.ts` * `.agents/skills/surfpool/resources/cheatcodes.md` * `.agents/skills/surfpool/resources/cli-reference.md` * `.agents/skills/surfpool/resources/github-repos.md` * `.agents/skills/surfpool/templates/Surfpool.toml` * `.agents/skills/surfpool/templates/test-setup.ts` * `.gitignore` * `AGENTS.md` * `Anchor.toml` * `README.md` * `codama.json` * `docs/architecture.md` * `docs/glossary.md` * `docs/resolution.md` * `docs/tokenomics.md` * `package.json` * `programs/opal/Cargo.toml` * `programs/opal/src/constants.rs` * `programs/opal/src/errors.rs` * `programs/opal/src/instructions/challenge_llm_resolution.rs` * `programs/opal/src/instructions/create_assertion.rs` * `programs/opal/src/instructions/decrement.rs` * `programs/opal/src/instructions/dispute_assertion.rs` * `programs/opal/src/instructions/finalize_llm_resolution.rs` * `programs/opal/src/instructions/finalize_undisputed.rs` * `programs/opal/src/instructions/finalize_vote_resolution_placeholder.rs` * `programs/opal/src/instructions/increment.rs` * `programs/opal/src/instructions/initialize.rs` * `programs/opal/src/instructions/initialize_protocol_config.rs` * `programs/opal/src/instructions/mod.rs` * `programs/opal/src/instructions/open_vote.rs` * `programs/opal/src/instructions/submit_mock_llm_resolution.rs` * `programs/opal/src/lib.rs` * `programs/opal/src/state/assertion.rs` * `programs/opal/src/state/counter.rs` * `programs/opal/src/state/llm_dispute.rs` * `programs/opal/src/state/llm_resolution_round.rs` * `programs/opal/src/state/mod.rs` * `programs/opal/src/state/protocol_config.rs` * `programs/opal/src/state/vote_dispute.rs` * `programs/opal/src/state/vote_resolution_round.rs` * `programs/opal/src/utils.rs` * `tests/opal.devnet.test.ts` * `tests/opal.test.ts` * `tsconfig.json` </details> <details> <summary>💤 Files with no reviewable changes (4)</summary> * programs/opal/src/instructions/decrement.rs * programs/opal/src/instructions/increment.rs * programs/opal/src/instructions/initialize.rs * programs/opal/src/state/counter.rs </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Contributor
Author
|
Key changes:
|
There was a problem hiding this comment.
♻️ Duplicate comments (3)
programs/opal/src/instructions/initialize_protocol_config.rs (1)
54-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-enable deployer authorization for protocol config initialization.
On Line 54-55, the check is still commented, so any signer can initialize the singleton config first and control protocol parameters.
Suggested fix
- // !TBD: PLACEHOLDER — uncomment before mainnet to restrict init to deployer. - // require!(ctx.accounts.authority.key() == crate::ID, OpalError::Unauthorized); + require!( + ctx.accounts.authority.key() == crate::ID, + OpalError::Unauthorized + );As per coding guidelines:
programs/opal/src/instructions/initialize_protocol_config.rs: "Uncomment therequire!(authority.key() == crate::ID)check ininitialize_protocol_configbefore mainnet deployment to enforce deployer auth".🤖 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 `@programs/opal/src/instructions/initialize_protocol_config.rs` around lines 54 - 55, Uncomment and re-enable the deployer authorization check inside initialize_protocol_config: restore the require!(ctx.accounts.authority.key() == crate::ID, OpalError::Unauthorized) line so only the program/deployer (crate::ID) can initialize the singleton protocol config; locate the require! invocation in initialize_protocol_config and remove the comment markers so the guard runs at runtime.docs/architecture.md (2)
15-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse “zero-padded fixed-size byte array” terminology instead of “null-terminated string.”
Line 15 is misleading for max-length payloads; integrators should decode these as fixed arrays and trim trailing zero bytes.
Doc patch
-- The statement lives onchain as a fixed-size byte array (null-terminated string, max 280 bytes). -- Auxiliary data lives offchain as plain text; only `auxiliary_hash` is stored onchain (max 128 bytes). +- The statement lives onchain as a zero-padded fixed-size byte array (max 280 bytes). +- Auxiliary data lives offchain as plain text; only `auxiliary_hash` is stored onchain as a zero-padded fixed-size byte array (max 128 bytes).🤖 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 `@docs/architecture.md` around lines 15 - 16, Replace the phrase "null-terminated string" in the onchain statement description with "zero-padded fixed-size byte array" and explicitly state that integrators must decode the field as a fixed-size byte array and trim trailing zero bytes (i.e., right-strip 0x00) when converting to text; also clarify the max size (280 bytes) remains the fixed width so consumers know to expect padding.
201-252:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd an explicit timeout recovery path for stalled escalations.
The documented flow still has no terminal transition that unwinds/refunds bonds when LLM submit, vote open, or vote finalize never occurs, which can strand funds in intermediate states.
🤖 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 `@docs/architecture.md` around lines 201 - 252, The flow lacks timeout recovery for stalled escalations—add explicit timeout/unwind transitions and a handler (e.g., cancel_escalation_on_timeout or reclaim_bonds_on_timeout) that can be invoked after configured deadlines to refund/unlock bonds and set terminal state; specifically: add timeout checks and new transitions for assertions stuck in ASSERTED_LLM (use llm_challenge_deadline), PENDING_VOTE and VOTING (use voting_deadline) that move the AssertionAccount to RESOLVED or a new CANCELLED terminal state, populate settlement fields on LlmDisputeAccount and VoteDisputeAccount, compute correctness/fees, and perform bond settlement exactly like finalize_llm_resolution / finalize_vote_resolution_placeholder / finalize_undisputed so funds are not stranded; wire these checks into the same places where deadlines are enforced and expose a permissioned or permissionless entrypoint as appropriate.
🧹 Nitpick comments (1)
programs/opal/src/instructions/open_vote.rs (1)
13-14: 💤 Low valueUnused
authoritysigner may confuse callers.The
authorityfield is declared as aSignerbut the handler never verifies it (line 38 notes auth is TBD). If the instruction is intentionally permissionless, consider removing the signer requirement to avoid confusion. If auth is planned, add a TODO to the accounts struct.🤖 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 `@programs/opal/src/instructions/open_vote.rs` around lines 13 - 14, The OpenVote accounts struct declares a Signer field named authority but the handler does not verify it (auth is TBD), which is confusing; either remove the authority: Signer<'info> field from the OpenVote struct to make the instruction permissionless, or if authority checks will be added later, keep the field and add a clear TODO comment above the authority field (and/or a short // TODO in the handler) stating that authority validation is pending and where it will be enforced; reference the OpenVote struct and the authority field so the change is localized and reviewers know intent.
🤖 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.
Duplicate comments:
In `@docs/architecture.md`:
- Around line 15-16: Replace the phrase "null-terminated string" in the onchain
statement description with "zero-padded fixed-size byte array" and explicitly
state that integrators must decode the field as a fixed-size byte array and trim
trailing zero bytes (i.e., right-strip 0x00) when converting to text; also
clarify the max size (280 bytes) remains the fixed width so consumers know to
expect padding.
- Around line 201-252: The flow lacks timeout recovery for stalled
escalations—add explicit timeout/unwind transitions and a handler (e.g.,
cancel_escalation_on_timeout or reclaim_bonds_on_timeout) that can be invoked
after configured deadlines to refund/unlock bonds and set terminal state;
specifically: add timeout checks and new transitions for assertions stuck in
ASSERTED_LLM (use llm_challenge_deadline), PENDING_VOTE and VOTING (use
voting_deadline) that move the AssertionAccount to RESOLVED or a new CANCELLED
terminal state, populate settlement fields on LlmDisputeAccount and
VoteDisputeAccount, compute correctness/fees, and perform bond settlement
exactly like finalize_llm_resolution / finalize_vote_resolution_placeholder /
finalize_undisputed so funds are not stranded; wire these checks into the same
places where deadlines are enforced and expose a permissioned or permissionless
entrypoint as appropriate.
In `@programs/opal/src/instructions/initialize_protocol_config.rs`:
- Around line 54-55: Uncomment and re-enable the deployer authorization check
inside initialize_protocol_config: restore the
require!(ctx.accounts.authority.key() == crate::ID, OpalError::Unauthorized)
line so only the program/deployer (crate::ID) can initialize the singleton
protocol config; locate the require! invocation in initialize_protocol_config
and remove the comment markers so the guard runs at runtime.
---
Nitpick comments:
In `@programs/opal/src/instructions/open_vote.rs`:
- Around line 13-14: The OpenVote accounts struct declares a Signer field named
authority but the handler does not verify it (auth is TBD), which is confusing;
either remove the authority: Signer<'info> field from the OpenVote struct to
make the instruction permissionless, or if authority checks will be added later,
keep the field and add a clear TODO comment above the authority field (and/or a
short // TODO in the handler) stating that authority validation is pending and
where it will be enforced; reference the OpenVote struct and the authority field
so the change is localized and reviewers know intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d7c78e1-ca6b-4e57-9f9a-e90e67937f35
📒 Files selected for processing (18)
AGENTS.mdREADME.mddocs/architecture.mdpackage.jsonprograms/opal/src/constants.rsprograms/opal/src/errors.rsprograms/opal/src/instructions/challenge_llm_resolution.rsprograms/opal/src/instructions/create_assertion.rsprograms/opal/src/instructions/dispute_assertion.rsprograms/opal/src/instructions/finalize_llm_resolution.rsprograms/opal/src/instructions/finalize_vote_resolution_placeholder.rsprograms/opal/src/instructions/initialize_protocol_config.rsprograms/opal/src/instructions/open_vote.rsprograms/opal/src/instructions/submit_mock_llm_resolution.rsprograms/opal/src/state/llm_resolution_round.rsprograms/opal/src/state/vote_resolution_round.rsprograms/opal/src/utils.rstests/opal.test.ts
✅ Files skipped from review due to trivial changes (1)
- programs/opal/src/instructions/dispute_assertion.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- programs/opal/src/state/llm_resolution_round.rs
- programs/opal/src/state/vote_resolution_round.rs
- programs/opal/src/constants.rs
- programs/opal/src/instructions/finalize_llm_resolution.rs
- tests/opal.test.ts
- programs/opal/src/instructions/challenge_llm_resolution.rs
- programs/opal/src/instructions/finalize_vote_resolution_placeholder.rs
This was referenced May 6, 2026
Merged
This was referenced May 16, 2026
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
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.
This pull request scaffolds the basic assertion flow with placeholders for llm resolution and vote resolution. Complete with tests and zero-copy accounts in anchor.
I am sorry to whoever has to review this PR, it's a monstrosity but ignoring the
package.lockand.agentsdir it should be about 3k LOC@mitgajera @takshakmudgal have a look
NOTE: We do have one flaky test at the time of writing this
Summary by CodeRabbit
New Features
Documentation
Tests