Skip to content

Reward authority rotation primitive (RewardPool)#254

Merged
rickyrombo merged 10 commits into
mainfrom
mjp-reward-pools-bundle
May 12, 2026
Merged

Reward authority rotation primitive (RewardPool)#254
rickyrombo merged 10 commits into
mainfrom
mjp-reward-pools-bundle

Conversation

@rickyrombo
Copy link
Copy Markdown
Contributor

@rickyrombo rickyrombo commented May 8, 2026

Goal

Add a CometBFT-side abstraction — RewardPool — so the set of eth addresses authorized to attest for programmatic rewards under a given Solana reward manager (RM) can be updated without reissuing the reward rows. Today every core_rewards row carries an immutable inline claim_authorities array, set at create time. Rotating an authority means deleting and recreating every row that references it.

A leaked launchpad-derived per-mint claim authority key currently retains permanent attestation rights on every reward under that mint until those rows are individually deleted and re-created. Programmatic-reward attestations have been disabled behind a kill switch (#215) since this gap was identified. This PR is the unblock.

What ships (logical chunks)

This PR bundles three sequenced changes that previously lived as separate stacked PRs (#222 / #225 / #228). They were merged onto one branch off main because reviewing them in isolation kept producing the same circular questions about cross-PR invariants. The commit history preserves the chunk boundaries.

1. Schema + migration (pkg/core/db/sql/migrations/00034_reward_pools.sql)

  • New table core_reward_pools (rewards_manager_pubkey TEXT PRIMARY KEY, authorities TEXT[], …). Pool identity is the Solana RM pubkey — there is no separate pool_id.
  • New column core_rewards.rewards_manager_pubkey (FK → core_reward_pools). The old claim_authorities column on core_rewards is dropped; reads now do LEFT JOIN core_reward_pools p and alias coalesce(p.authorities, '{}') as claim_authorities. Existing read queries don't notice.
  • Seed table launchpad_authority_rm mapping each known per-mint claim authority (lowercased eth) → its Solana RM. Values are produced by running DeriveEthAddressForMint(domain="claimAuthority", secret, mint) for every launchpad mint init event. Used by the migration backfill and (transitively) by the wire-compat replay path.
  • Backfill: per RM, take the UNION of authorities across every reward whose claim_authorities contains any per-mint key for that RM. Rows whose authorities don't match any seed entry stay with rewards_manager_pubkey = NULL and are unrecoverable — see "Gaps."

2. CometBFT transactions (pkg/core/server/reward_pools.go, pkg/core/server/rewards_legacy.go, proto)

  • Body+signature envelope: Tx { TxBody body; signatures[] } — signature lifted off individual actions onto a wrapping envelope (cosmos-style). This is wire-incompatible with the pre-pool shape, which had deadline + signature embedded inside each action.
  • Two new pool-management actions: CreateRewardPool and SetRewardPoolAuthorities. Both gated by:
    • validateRewardsManagerPubkey (must be valid base58, exactly 32 bytes, not whitespace, not the AUDIO RM).
    • validateAuthorityList (non-empty, every entry valid eth address).
    • For Create: signer ∈ initial authorities. For Set: signer ∈ current pool authorities.
  • CreateReward proto: claim_authorities, deadline_block_height, signature removed (tags 4-6 reserved); rewards_manager_pubkey at tag 7. New CreateReward requires an existing pool. validateCreateReward checks signer ∈ pool.authorities.
  • Wire-compat: legacy-shape bytes are rejected at validate (CheckTx + ProcessProposal) so no new legacy txs can be accepted, but applied at finalize so block-sync replay reproduces historical state. tryParseLegacyReward recovers the original tag 1000/1001 bytes via proto-go's preserved unknown fields.

3. Validator endpoint cutover (pkg/core/server/connect.go)

  • GetRewardAttestation: restored from Temporarily block artist-coin attestations at the node #215. Auth check uses dbReward.ClaimAuthorities, which is sourced from coalesce(p.authorities, '{}') — i.e., the current pool membership. Rotating an authority out via SetRewardPoolAuthorities immediately revokes their ability to authenticate claims.
  • GetRewardSenderAttestation / GetDeleteRewardSenderAttestation: per-RM gate. If a pool exists for the requested RM, sign iff addr ∈ pool.authorities (Add) or addr ∉ pool.authorities (Delete — the rotation-out signal). If no pool exists, fall through to the validator/AAO trust set only for the configured AUDIO RM; every other no-pool RM returns InvalidArgument.

Tradeoffs and key decisions

RewardClaim.Compile does not bind to RewardAddress

The proto field exists, and Compile has a 3-piece RewardAddress:RewardID:Specifier disbursement_id branch — but the bytes Compile produces are exactly what the Solana reward manager program reconstructs in evaluate_attestations, and that program expects the 2-piece RewardID:Specifier form. Populating RewardAddress here would break on-chain signature verification.

Consequence: cross-reward replay protection relies on Specifier being disbursement-unique (per recipient + per event), which is the existing client contract. A signature for (RewardID, Specifier, recipient, amount, authority) is valid for any reward sharing all five with a pool containing authority. In practice Specifier collisions across rewards don't occur; documented inline at the RewardClaim construction site so a future reader doesn't "fix" it.

AUDIO stays on the validator/AAO trust set

AUDIO is intentionally outside the pool primitive. validateRewardsManagerPubkey actively refuses to create a pool whose RM matches the configured AUDIO RM. Without this denylist an attacker could create a pool for the AUDIO RM with their own initial authorities and have validators sign AUDIO sender attestations on their behalf.

The AUDIO RM constants live in pkg/core/config/rewards.go per env. Stage is intentionally empty (staging doesn't run a real AUDIO mint, so the denylist would never legitimately fire). Dev and Prod are populated.

Privilege escalation in legacy finalizeCreateReward

Reverted in development: an early version of the legacy create path upserted authorities into the resolved real-RM pool. Because legacy validateCreateReward only checked the tx signature (not signer ∈ pool.authorities), an attacker could submit a legacy CreateReward with [legitimate_per_mint_key, attacker_key] and inject attacker_key into the legitimate pool. The current code uses lookup-only: resolve the RM, bind the reward to the existing pool if one exists, leave NULL otherwise. Pool authorities mutate only via CreateRewardPool / SetRewardPoolAuthorities.

Migration UNION ↔ replay UNION (apphash determinism)

Earlier review found a divergence: migration step 1 produced pool.authorities = UNION(every legacy reward's authorities), but the replay-time upsert was a hard overwrite, producing pool.authorities = last-replayed-reward.authorities. An in-place-upgraded node and a from-genesis-syncing node would end up with different DB state → different validation outcomes → apphash divergence.

Fixed: UpsertLegacyReplayRewardPool (renamed from the misleading UpsertSyntheticRewardPool) now uses true union semantics in DO UPDATE. Today's data has at most one authority per reward, so the bug doesn't manifest currently — this is insurance against multi-authority drift.

Body+signature envelope is wire-incompatible with the pre-pool shape

This isn't a soft cutover. The pre-pool wire shape has no field at tag 1 (where the new Body lives), so the new proto unmarshals legacy bytes with Body == nil. The wire-compat layer (rewards_legacy.go) is the bridge for replay; live legacy traffic is rejected. Clients (SDK, API repo) must be updated in lockstep — the SDK update is in this PR; the API repo is NOT (deferred per stack plan).

Down migration is best-effort

core_rewards.claim_authorities is restored from the pool's authorities on a Down. Rows with rewards_manager_pubkey = NULL (no launchpad match) lose their authorities permanently — there's no pool to restore from. Documented in the migration; production rollback would happen at the chain level, not via SQL Down.

Gaps / things to know

  • launchpad_authority_rm lifetime. This table is queried at replay time by finalizeLegacyCreateReward. It must remain populated as long as legacy bytes can be replayed. PR Remove legacy reward wire-compat layer (post-restart cleanup) #232 (separate, post-restart cleanup) drops the table after the wire-compat layer is removed. Do not drop sooner.
  • Specifier uniqueness is the only cross-reward replay defense. See "RewardClaim.Compile" above.
  • AUDIO denylist is env-dependent. AudioRewardsManagerPubkey() reads GetRuntimeEnvironment(). A node misconfigured to run with env=dev against prod data would skip the prod AUDIO denylist. Not currently asserted at startup.
  • State-sync table dump still includes launchpad_authority_rm. Required while wire-compat is in place; PR Remove legacy reward wire-compat layer (post-restart cleanup) #232 removes it together with the table drop.
  • "Empty" non-AUDIO RM handling. senderGateForRM returns ErrSenderGateUnknownRM (mapped to InvalidArgument) for any RM with no pool that isn't the AUDIO RM. Earlier behavior would silently fall through to validator/AAO, which is more permissive than desired now that pools exist.
  • API repo is NOT part of this PR. Downstream apps/api create-reward call sites need to issue CreateRewardPool before CreateReward for new mints. Until they do, new launchpad mints will fail at validateCreateReward's pool existence check. Deferred intentionally; will land separately.

Deployment

Test plan

  • go test ./pkg/core/server/ ./pkg/rewards/ ./pkg/common/ — unit tests for pool tx validation, AUDIO denylist, sender gate, legacy decode/replay, attestation flow.
  • Integration tests in pkg/integration_tests/13_reward_pools_test.go (new) and 12_rewards_test.go (updated) — round-trip pool creation, authority rotation, and attestation gating against a live devnet.
  • Apply migration against a snapshot of staging (or a fixture with rewards under multiple RMs) and confirm: one pool per RM, pool authorities = UNION of contributing rewards' authorities, every row has a non-NULL FK or is in the documented "no launchpad match" bucket.
  • Verify validateCreateRewardPool refuses the configured Prod/Dev AUDIO RM.
  • Verify GetRewardSenderAttestation returns InvalidArgument for an arbitrary RM with no pool that isn't AUDIO.
  • Manual: rotate an authority via SetRewardPoolAuthorities, re-attempt GetRewardAttestation with the rotated-out key — expect PermissionDenied. Re-attempt with the rotated-in key — expect success.

🤖 Generated with Claude Code

rickyrombo and others added 2 commits May 8, 2026 01:51
Three logical chunks bundled onto a single branch off main:

PR1 — Schema (mjp-reward-pools-schema):
  - New core_reward_pools table keyed by Solana RM pubkey, with a
    text[] authorities column (gin-indexed for @> containment).
  - launchpad_authority_rm seed table mapping every known launchpad-
    derived per-mint claim authority → its Solana reward manager
    state account. Used by both the migration backfill and PR2's
    wire-compat replay logic.
  - core_rewards.rewards_manager_pubkey FK column; claim_authorities
    column dropped (reads now alias coalesce(p.authorities, '{}')
    via LEFT JOIN on core_reward_pools).
  - Backfill creates one pool per RM (per-RM authority union across
    all rewards referencing it via launchpad lookup). Rows whose
    authorities don't match any launchpad RM stay NULL — there are
    no synthetic mig_<md5> identifiers.
  - Live finalizeCreateReward (legacy proto shape, brief PR1-only
    window) does launchpad lookup → bind to existing pool only;
    never upserts. NULL fallback if no match or pool missing.

PR2 — CometBFT transactions (mjp-reward-pools-tx):
  - New body+signature envelope: Tx { TxBody body; signatures[] }.
    Reward and RewardPool messages move to the new shape.
  - CreateRewardPool / SetRewardPoolAuthorities txs gated by
    real-RM-shape pubkey + signer ∈ current pool authorities.
  - CreateReward proto reserves tags 4-6 (former claim_authorities,
    deadline, signature) and uses tag 7 for rewards_manager_pubkey.
    DeleteReward reserves tags 2-3.
  - Wire-compat layer (rewards_legacy.go): legacy bytes are
    REJECTED at CheckTx/ProcessProposal (no new legacy txs
    accepted) but ACCEPTED at FinalizeBlock for block-sync replay
    of historical chain state. Replay uses launchpad lookup to
    bind legacy rewards to the same RM the migration produced.
  - Defense-in-depth re-validation at finalize for both pool txs
    (block-sync replay skips ProcessProposal / CheckTx).

PR3 — Validator endpoint cutover (mjp-reward-pools-endpoints):
  - GetRewardAttestation restored from the #215 kill-switch. Auth
    check uses dbReward.ClaimAuthorities, which is sourced from
    coalesce(p.authorities, '{}') — so rotating an authority out
    via SetRewardPoolAuthorities immediately revokes attestation
    rights. RewardClaim.RewardAddress is intentionally NOT set
    (Solana reward manager program expects 2-piece RewardID:
    Specifier disbursement_id).
  - GetRewardSenderAttestation / GetDeleteRewardSenderAttestation
    dispatch by RM: pool-gated if pool exists, else fall back to
    the legacy validator/AAO trust set (AUDIO path).
  - AUDIO RM denylist on validateRewardsManagerPubkey: prevents an
    attacker from creating a pool for the AUDIO RM and inheriting
    AUDIO sender attestations. Per-env constants in
    pkg/core/config/rewards.go (dev/prod populated; stage left
    empty intentionally).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three review-driven fixes on the bundle branch:

1. Replay/migration apphash divergence (#1).
   UpsertSyntheticRewardPool was a hard overwrite, which produced
   pool.authorities = last-replayed-reward.authorities on a from-genesis
   block-sync — diverging from the migration backfill, which UNIONs
   authorities across every legacy reward referencing the RM. Production
   data has at most one authority per reward today, so the bug doesn't
   currently manifest, but it's cheap insurance against future drift
   (multi-authority rewards, debug keys, etc.). The DO UPDATE clause now
   unions existing pool authorities with the incoming set.

   Renamed the query to UpsertLegacyReplayRewardPool to reflect its
   actual (and only) caller — the mig_<md5> shape was already gone (#5).

2. senderGateForRM AUDIO-only fallback (#2).
   The legacy validator/AAO trust set used to be the fallback for ANY
   RM without a pool. That was a quietly-permissive seam — any caller
   could request validator-signed attestations for an arbitrary unknown
   RM. Now the fallback applies only when the requested RM equals the
   configured AUDIO RM; every other no-pool RM gets
   ErrSenderGateUnknownRM, which the handlers map to InvalidArgument.

3. Stale doc comment in rewards_legacy.go (#6) saying the file did
   "synthetic-pool fallback for create" — predates the mig_<md5>
   removal. Updated to describe the launchpad-lookup behavior.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a CometBFT-side RewardPool primitive keyed by Solana rewards_manager_pubkey so programmatic reward claim authorities can be rotated without rewriting reward rows, while keeping historical chain replay deterministic via a legacy wire-compat layer.

Changes:

  • Introduces core_reward_pools + migration/backfill, moves rewards’ authorities to pool membership, and adds state-sync coverage for new tables.
  • Adds a new body+signature transaction envelope, reward-pool management transactions (CreateRewardPool, SetRewardPoolAuthorities), and legacy reward replay decoding/signature recovery.
  • Restores and updates validator Connect endpoints to gate attestations/sender attestations by pool membership (with AUDIO remaining on the validator/AAO trust model), plus SDK + integration test updates.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
proto/core/v1/types.proto Adds RewardBody/RewardMessage envelope, RewardPool* messages, legacy reward wire types, and reward-pool RPC types.
proto/core/v1/service.proto Adds GetRewardPool RPC.
pkg/api/core/v1/types.pb.go Regenerated protobuf Go types for new reward/pool envelopes and RPC types.
pkg/api/core/v1/service.pb.go Regenerated CoreService protobuf bindings to include GetRewardPool.
pkg/api/core/v1/v1connect/service.connect.go Regenerated Connect client/server bindings to include GetRewardPool.
pkg/core/db/sql/migrations/00033_reward_pools.sql Creates core_reward_pools + launchpad_authority_rm, backfills pools/RM FKs, drops inline claim_authorities.
pkg/core/db/sql/reads.sql Updates reward reads to join pools and adds reward-pool + launchpad RM lookup queries.
pkg/core/db/sql/writes.sql Updates reward writes to store rewards_manager_pubkey and adds pool insert/update + legacy replay upsert.
pkg/core/db/reads.sql.go Regenerated sqlc read layer for new JOIN-based reward reads and pool queries.
pkg/core/db/writes.sql.go Regenerated sqlc write layer for RM-based rewards and pool mutations.
pkg/core/db/models.go Adds CoreRewardPool / LaunchpadAuthorityRm models and updates reward model fields.
pkg/core/server/state_sync.go Includes core_reward_pools and launchpad_authority_rm in PG dump state sync.
pkg/core/server/abci.go Wires reward-pool tx validation/finalization into ABCI flow.
pkg/core/server/rewards.go Switches reward tx validation/finalization to body+signature envelope; adds deadline signer recovery and legacy dispatch.
pkg/core/server/rewards_legacy.go Implements legacy reward decode + signer recovery + replay-time RM/pool binding.
pkg/core/server/rewards_legacy_test.go Tests unknown-field legacy decode + validate-time legacy rejection.
pkg/core/server/reward_pools.go Adds reward-pool tx validation/finalization, authority validation, and RM pubkey validation/denylist.
pkg/core/server/reward_pools_test.go Unit tests for RM pubkey shape checks and AUDIO denylist behavior.
pkg/core/server/connect.go Restores reward attestation, normalizes GetRewards authority lookup, adds GetRewardPool, and implements per-RM sender attestation gating.
pkg/core/config/rewards.go Adds per-env AUDIO RM pubkeys and accessor used for denylisting pool creation.
pkg/common/proto.go Implements deterministic-proto signing/recovery helpers for envelope bodies.
pkg/common/proto_test.go Adds tests for ProtoSign/ProtoRecover correctness and oneof discrimination.
pkg/common/legacy_reward_signing.go Reintroduces legacy reward signing helpers for historical replay signer recovery.
pkg/common/reward_signing.go Removes pre-envelope reward signing helpers.
pkg/rewards/reward_pool.go Adds canonicalization helper for pool authority sets.
pkg/rewards/reward_pool_test.go Tests canonical authority normalization behavior.
pkg/sdk/rewards/rewards.go Updates SDK to sign/send envelope bodies and adds pool + sender-attestation helpers.
pkg/integration_tests/12_rewards_test.go Updates integration tests to create pools and restores attestation assertions.
pkg/integration_tests/13_reward_pools_test.go Adds integration test coverage for pool lifecycle, rotation, and sender attestation gating.
examples/rewards/main.go Updates example to create a pool first and then create a reward using rewards_manager_pubkey.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread proto/core/v1/types.proto Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/core/server/connect.go Outdated
Closes the pool-creation frontrunning vector. Today's
validateCreateRewardPool only requires signer ∈ initial_authorities,
which an attacker satisfies trivially by listing themselves. After a
new reward manager is initialized on Solana, an observer who watches
init events can race the legitimate launchpad operator's
CreateRewardPool and register a pool with attacker-chosen authorities;
the legitimate operator is then locked out (PK conflict on
rewards_manager_pubkey), and the attacker controls every reward and
sender attestation under the RM.

Defense rests on a property of the existing system: the Solana
rewardManagerState account is a deterministic ed25519 keypair, derived
by the launchpad relay as

  Keypair.fromSeed(sha256(launchpadDeterministicSecret ||
                          'audius-launchpad' ||
                          'reward-manager' ||
                          mint))

(see apps/.../solana-relay/.../launchpad/launch_coin.ts). The
launchpad has the secret and can re-derive the keypair at will; an
attacker who lacks the secret cannot. The 32-byte rewardManagerState
public key IS what cometbft has been carrying as
rewards_manager_pubkey — so we already have an ed25519 verification
key in hand at validate time.

This commit:

  1. Adds CreateRewardPool.rm_owner_signature (proto tag 3, bytes).
  2. Defines a canonical signing payload in pkg/rewards:
       "audius:create-reward-pool:" + chain_id + ":" +
       rm_pubkey_b58 + ":" + sorted_lowercased_authorities.join(",")
     and a SignCreateRewardPool helper for client-side use.
  3. validateCreateRewardPool and finalizeCreateRewardPool each call
     verifyRewardPoolOwnerSignature, which decodes rm_pubkey from
     base58 and runs ed25519.Verify against the canonical payload.
     Defense-in-depth at finalize matches the existing pattern for
     replay-time invariants.
  4. Updates SDK example (examples/rewards/main.go) and integration
     tests to populate the signature. New unit tests cover positive
     verification, canonicalization invariance, foreign-keypair
     rejection, cross-chain replay, mismatched authorities, malformed
     signature length, and rm_pubkey shape errors.

The existing signer ∈ initial_authorities check is retained alongside
the new ed25519 gate. They're independent: the ed25519 sig proves
control of the RM keypair (frontrunning defense); the membership
check enforces the existing "you can't create a pool you have no
membership in" property. Operationally, the launchpad relay holds
both the per-mint claim authority eth key (envelope signer + the only
initial authority) and the RM ed25519 keypair, so producing both
signatures is symmetric.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
raymondjacobson added a commit that referenced this pull request May 11, 2026
State sync reliability fixes are now merged (PR #253, #254, #256).
Restoring the edge tag job so nodes on auto-upgrade resume tracking
the latest main.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
rickyrombo and others added 3 commits May 12, 2026 11:02
Restructuring on top of the previous commit: the ed25519
rm_owner_signature moves from CreateRewardPool.rm_owner_signature (tag
3, signing a custom canonical string) to
RewardPoolMessage.rm_owner_signature (envelope-level, signing the same
ProtoMarshal(body) bytes the secp256k1 envelope signature covers).

Why:

  - One encoding to maintain instead of two. Cross-language clients
    (the TS launchpad relay) now sign the same bytes for both
    signatures; no separate domain-separated string format to keep in
    sync.
  - Body bytes implicitly cover deadline_block_height + the action
    oneof discriminator. The earlier custom string didn't include
    deadline; stale-deadline replay was technically possible (though
    blocked by pool PK uniqueness).
  - Future fields added to RewardPoolBody / CreateRewardPool are
    automatically covered without revving the signing scheme.

Not included: chain_id in the body. Cross-chain replay isn't a
concrete threat — each environment's launchpad uses a different
deterministic secret, so the same rewards_manager_pubkey cannot be
derived on more than one chain. A captured CreateRewardPool replayed
on another chain refers to an RM that doesn't exist there.

Other changes:

  - pkg/common.ProtoSignableBytes (new): exports the deterministic-
    marshal helper so verifyRewardPoolOwnerSignature can hash the
    same bytes ProtoSign / ProtoRecover use.
  - SDK signAndSendRewardPool takes an rmOwnerSig parameter; the
    CreateRewardPool wrapper accepts an ed25519.PrivateKey and signs
    body bytes locally. SetRewardPoolAuthorities passes nil — rotation
    is gated by current pool authorities, no RM signature needed.
  - Removed pkg/rewards.SignCreateRewardPool /
    CanonicalCreateRewardPoolPayload / CreateRewardPoolOwnerSignatureDomain
    — replaced by the body-bytes signing path.
  - Updated unit tests, integration tests, and example to populate
    rmKey at the SDK call site rather than constructing a signed
    message struct.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
main shipped a different 00033 (drop_redundant_tx_hash_index, #205)
while this branch was open. Bump ours to 00034 to keep migration
ordering unambiguous; content is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 30 changed files in this pull request and generated 7 comments.

Files not reviewed (5)
  • pkg/api/core/v1/service.pb.go: Language not supported
  • pkg/api/core/v1/v1connect/service.connect.go: Language not supported
  • pkg/core/db/models.go: Language not supported
  • pkg/core/db/reads.sql.go: Language not supported
  • pkg/core/db/writes.sql.go: Language not supported

Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/sdk/rewards/rewards.go
Comment thread pkg/core/server/connect.go Outdated
Comment thread pkg/core/server/connect.go
Comment thread pkg/core/server/connect.go
Comment thread pkg/core/server/connect.go
Comment thread pkg/core/db/sql/migrations/00034_reward_pools.sql Outdated
rickyrombo and others added 3 commits May 12, 2026 09:26
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Six Copilot-flagged items from the latest review:

1. Proto signature comments now spell out the actual pre-hashing:
   - RewardMessage.signature: secp256k1 over sha256(body bytes).
   - RewardPoolMessage.signature: same.
   - RewardPoolMessage.rm_owner_signature: ed25519 over body bytes
     directly (ed25519 hashes internally — do NOT pre-hash).
   Lets non-Go clients reproduce signatures without reading
   pkg/common/crypto.go.

2. Split validateRewardsManagerPubkey:
   - validateRewardsManagerPubkeyShape (new): non-empty, no whitespace,
     base58, 32 bytes. Pure shape. For read paths and rotation paths.
   - validateRewardsManagerPubkey (existing): shape + AUDIO denylist.
     Only for write paths (CreateRewardPool, CreateReward).

   Switched call sites:
   - validateSetRewardPoolAuthorities / finalizeSetRewardPoolAuthorities
     → Shape. SetAuthorities targets an existing pool; AUDIO has no
     pool by construction, so checkPoolAuthorization surfaces the case
     as "pool not found" rather than the misleading "is reserved".
   - GetRewardPool → Shape. Probing GetRewardPool(AudioRM) now returns
     a clean NotFound instead of InvalidArgument.
   - GetRewardSenderAttestation /
     GetDeleteRewardSenderAttestation → add Shape validation up front
     so malformed pubkeys return a clear InvalidArgument instead of
     falling through to ErrSenderGateUnknownRM (which is for valid-
     shape-but-unmapped RMs).

3. Removed the stale "chain_id is covered by signed body bytes"
   reference in validateCreateRewardPool's comment — the body
   doesn't carry chain_id, and that's intentional (cross-chain replay
   isn't a threat because per-env launchpad secrets prevent the same
   rewards_manager_pubkey from existing on more than one chain).

4. SDK CreateRewardPool now validates rmKey length up front and
   returns a typed error instead of panicking inside ed25519.Sign for
   callers that pass nil / hex-decode-wrong / public-key-by-mistake.

5. GetRewardAttestation now TrimSpaces eth_recipient_address,
   reward_address, and claim_authority at the boundary so
   surrounding whitespace returns a clean InvalidArgument here
   instead of a confusing hex-decode error deeper in
   RewardClaim.Compile.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@raymondjacobson raymondjacobson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a case where two create reward pool TXs exist in the same block and the second fails to finalize. Do we need an ON CONFLICT DO NOTHING ?

Comment thread examples/rewards/main.go Outdated
Comment thread pkg/core/config/rewards.go Outdated
Comment thread pkg/core/db/sql/migrations/00034_reward_pools.sql Outdated
Comment thread pkg/core/server/connect.go Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/integration_tests/13_reward_pools_test.go Outdated
Six items from raymondjacobson:

1. examples/rewards/main.go: drop REWARDS_MANAGER_SECRET_HEX env var
   and generate a fresh ed25519 keypair inline. Strip the explainer
   comments — the simpler example is self-documenting.

2. pkg/core/config/rewards.go: remove the staging-specific AUDIO RM
   constant and the long comment about why staging is empty. The
   AudioRewardsManagerPubkey() switch no longer special-cases stage,
   so staging falls through to "" via the default branch, which the
   denylist treats as "no enforcement." The reward_pools_test save/
   restore no longer touches StageAudioRewardsManagerPubkey.

3. 00034_reward_pools.sql backfill comment: drop "leaked-key" framing,
   replace with neutral "additional entries."

4 + 6. Sweep PR1/PR2/PR3/PR #225 references out of all bundle code
   and comments — these labeled stacked-PR boundaries that no longer
   exist now that the work is bundled. Phrasing now describes what
   the code does, not which PR introduced it. Touched: connect.go,
   reward_pools.go, rewards.go, rewards_legacy.go, reads.sql,
   migration, proto, integration test.

5. reward_pools.go: drop the case-insensitive contains() helper and
   use slices.Contains across all call sites. Pool authorities are
   already canonicalized (lowercase) on write via
   CanonicalAuthorities, so callers just lowercase the needle. Removes
   ~10 lines and a custom helper in favor of stdlib.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 30 changed files in this pull request and generated 2 comments.

Files not reviewed (5)
  • pkg/api/core/v1/service.pb.go: Language not supported
  • pkg/api/core/v1/v1connect/service.connect.go: Language not supported
  • pkg/core/db/models.go: Language not supported
  • pkg/core/db/reads.sql.go: Language not supported
  • pkg/core/db/writes.sql.go: Language not supported

Comment on lines +307 to +331
// Generate a key for creating the reward. Note this isn't the pool
// authority; the pool is created via authority's SDK so the envelope
// signer is in the initial authority list.
creatorKey, err := crypto.GenerateKey()
if err != nil {
t.Fatalf("Failed to generate creator key: %v", err)
}
creator := sdk.NewOpenAudioSDK(nodeUrl)
creator.SetPrivKey(creatorKey)

// Create a reward with specific amount
reward, err := creator.Rewards.CreateReward(ctx, &v1.CreateReward{
RewardId: "amount_test",
Name: "Amount Test Reward",
Amount: 100, // Fixed amount
ClaimAuthorities: []*v1.ClaimAuthority{
{Address: authorityAddr, Name: "Test Authority"},
},
DeadlineBlockHeight: 999999,
})
// Create a pool that names authority + a reward bound to it.
// Authority signs the envelope (it's the lone initial authority);
// the RM keypair signs the inner rm_owner_signature.
rmPubkey, rmPriv := freshRewardManager(t)
if _, err := authority.Rewards.CreateRewardPool(ctx, &v1.CreateRewardPool{
RewardsManagerPubkey: rmPubkey,
Authorities: []string{authorityAddr},
}, rmPriv, 999999); err != nil {
t.Fatalf("Failed to create pool: %v", err)
}
// authority is the only pool member, so it must create the reward
// (validateCreateReward gates on signer ∈ pool.authorities). The
// `creator` SDK above is unused under the pool model.
_ = creator
reward, err := authority.Rewards.CreateReward(ctx, &v1.CreateReward{
Comment on lines +146 to +163
func (r *Rewards) CreateRewardPool(ctx context.Context, msg *v1.CreateRewardPool, rmKey ed25519.PrivateKey, deadlineBlockHeight int64) (string, error) {
// ed25519.Sign panics on wrong-length keys; surface as a typed error
// instead so callers passing nil / hex-decoded-wrong / public-key-by-
// mistake see a recoverable failure rather than a runtime crash.
if len(rmKey) != ed25519.PrivateKeySize {
return "", fmt.Errorf("rmKey is %d bytes; want ed25519 private key (%d bytes)", len(rmKey), ed25519.PrivateKeySize)
}
body := &v1.RewardPoolBody{
DeadlineBlockHeight: deadlineBlockHeight,
Action: &v1.RewardPoolBody_Create{Create: msg},
}
bodyBytes, err := common.ProtoSignableBytes(body)
if err != nil {
return "", err
}
rmSig := ed25519.Sign(rmKey, bodyBytes)
return r.signAndSendRewardPool(ctx, body, rmSig)
}
@rickyrombo rickyrombo merged commit e499df4 into main May 12, 2026
7 of 8 checks passed
@rickyrombo rickyrombo deleted the mjp-reward-pools-bundle branch May 12, 2026 19:02
rickyrombo added a commit that referenced this pull request May 12, 2026
Ships PR #254. Adds CometBFT-side `RewardPool` so the eth addresses
authorized to attest for programmatic rewards under a Solana RM can be
updated without reissuing reward rows. Unblocks the artist-coin
attestation kill-switch from v1.2.11/#215.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
rickyrombo added a commit to AudiusProject/api that referenced this pull request May 14, 2026
OpenAudio/go-openaudio cut v1.2.13 (shipping the RewardPool
primitive merged in OpenAudio/go-openaudio#254). Replace the
pseudo-version we'd pinned to the merge commit with the clean tag.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants