Skip to content

Add reward pool cometbft transactions (PR 2 of 3) - #225

Closed
rickyrombo wants to merge 5 commits into
mjp-reward-pools-schemafrom
mjp-reward-pools-tx
Closed

Add reward pool cometbft transactions (PR 2 of 3)#225
rickyrombo wants to merge 5 commits into
mjp-reward-pools-schemafrom
mjp-reward-pools-tx

Conversation

@rickyrombo

@rickyrombo rickyrombo commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds two cometbft transactions for managing reward pools (CreateRewardPool, SetRewardPoolAuthorities) and gates CreateReward on pool membership. Stacked on top of #222 (PR 1 — schema). PR 3 (#228) cuts the validator endpoints over to per-RM pool gating.

This PR also folds in:

  • A refactor of the signing scheme — ProtoSign / ProtoRecover over a body+signature envelope, replacing the per-action custom canonical formats.
  • A wire-compat layer that lets the new binary apply pre-rollout reward txs during block-sync-from-genesis.

Per-RM pool primitive

The pool's identity IS the Solana reward manager pubkey (rewards_manager_pubkey). There is no separate "pool address" concept — pools and RMs are in 1:1 correspondence by construction.

PR3's sender-attestation gate collapses to a trivial lookup:

sign(prefix, RM, addr) iff:
  pool := GetRewardPool(RM)
  pool != nil AND addr ∈ pool.authorities

There is no field to misset, no binding to forget, no risk of "pool member of X" being treated as "authorized over Y." A pool exists for an RM, or it doesn't. AUDIO is naturally outside the pool surface — no CreateRewardPool will ever be issued for the AUDIO RM (PR 3 enforces this via a denylist), so PR 3's gate falls through to the existing validator/AAO logic.

rewards_manager_pubkey is validated as base58 32 bytes.

Wire shape

message RewardMessage      { RewardBody body;     string signature; }
message RewardPoolMessage  { RewardPoolBody body; string signature; }

message RewardBody {
  int64 deadline_block_height = 1;
  oneof action { CreateReward create = 1000; DeleteReward delete = 1001; }
}

message RewardPoolBody {
  int64 deadline_block_height = 1;
  oneof action {
    CreateRewardPool create = 2000;
    SetRewardPoolAuthorities set_authorities = 2001;
  }
}

The signature signs the deterministic protobuf marshaling of the body. Body and signature are siblings in the envelope (cosmos Tx { TxBody body; signatures[] } pattern); the body never carries its own signature, so there's no chicken-and-egg and no field-clearing dance.

Pool actions

  • CreateRewardPool { rewards_manager_pubkey, authorities[] } — caller specifies the Solana RM pubkey the pool will govern. Validated as base58 32 bytes. Same-RM in-block collisions surface as a PK violation at finalize and fail the second tx. Signer must be in the initial authorities list, and every authority must be a valid eth hex address.
  • SetRewardPoolAuthorities { rewards_manager_pubkey, authorities[] } — replaces the pool's authority set wholesale. Signer must be in the current authorities; new list must be non-empty and contain only valid eth addresses.

CreateReward gate

CreateReward { reward_id, name, amount, rewards_manager_pubkey } requires an existing first-class pool. The recovered signer must be a current member of pool.authorities; the reward inherits attestation rights from the pool going forward. Inline claim_authorities is dropped (tag 4 reserved).

Wire-compat for historical replay

The body+signature envelope is wire-incompatible with the pre-pool network's RewardMessage shape: legacy txs already on chain decode into the new shape with Body == nil. To keep block-sync-from-genesis working:

  • LegacyRewardMessage / LegacyCreateReward / LegacyDeleteReward proto types pin the original wire shape (oneof at tags 1000/1001 with deadline + signature embedded inside each action).
  • pkg/common/legacy_reward_signing.go reproduces the original sha256-over-pipe-delimited-canonical-string signing scheme for signer recovery.
  • tryParseLegacyReward (rewards_legacy.go): when the new RewardMessage decodes with Body == nil, the original tag-1000/1001 bytes survive as proto unknown fields. Re-marshal + decode as LegacyRewardMessage recovers them.
  • finalizeLegacyCreateReward resolves the reward's RM by looking up any of its inline claim_authorities in PR 1's launchpad_authority_rm table and writes a real RM-bound pool — matching the migration's backfill exactly so historical replay and the migration agree on the resulting apphash. Rewards whose authorities don't appear in the launchpad mapping (test fixtures, abandoned rewards) are inserted with NULL rewards_manager_pubkey.

Asymmetric gate: legacy bytes are REJECTED at validate-time (CheckTx + ProcessProposal) and ACCEPTED at finalize-time (FinalizeBlock). Live legacy traffic cannot enter mempools or be voted into blocks; historical legacy txs in already-committed blocks still apply during sync. Legacy CreateReward was permissionless by design (no signer-membership check on inline claim_authorities); accepting live legacy txs would let an attacker craft legacy bytes with arbitrary inline authorities and bypass the pool gate. Block sync only invokes FinalizeBlockProcessProposal is for live consensus, not replay.

Authorization re-checks at finalize

validateBlockTxs runs against pre-block state, but FinalizeBlock applies block txs sequentially, so an earlier tx in the same block can rotate the signer out before a later one runs. finalizeCreateReward, finalizeDeleteReward, and finalizeSetRewardPoolAuthorities re-fetch the pool / reward via s.getDb() (transactional, sees prior in-block changes) and re-check authorization. Extracted as checkPoolAuthorization so the rule lives in one place.

Defense-in-depth fixes

  • validateAuthorityList: rejects non-eth-address strings on CreateRewardPool / SetRewardPoolAuthorities. Without this, a current authority could rotate the pool to ["not-an-address"], which passes canonicalization but leaves no key able to satisfy checkPoolAuthorization — every attached reward becomes permanently unclaimable.
  • GetRewards lowercases the caller-supplied claim_authority to match the canonicalized stored values.
  • validateCreateReward distinguishes pgx.ErrNoRows from transient DB failures.

Why per-RM pool identity (vs. caller-chosen pool addresses)

Earlier drafts let callers pick an arbitrary pool_address and added a separate rewards_manager_pubkey field for the Solana binding. That introduced a foot-gun: the binding could be set wrong (or not set at all), and PR 3's sender-attestation gate would have to reason about pool↔RM associations that the schema didn't structurally enforce. Collapsing pool identity onto the RM pubkey eliminates the foot-gun entirely.

Why the body+signature shape (vs. flat envelope-with-signature)

Splitting into body + signature:

  • Eliminates the field-clearing hack: the body literally has no signature field.
  • Makes the layering legible: the body is what's signed (data + deadline), the envelope is transport (body + signature).
  • Cross-action replay protection still works: the body's oneof field tag discriminates Create from SetAuthorities by construction.

Why deterministic protobuf (vs. custom canonical formats)

Replaces the per-action CreateDeterministicXxxData helpers with a single ProtoSign / ProtoRecover pair using proto.MarshalOptions{Deterministic: true}. proto3's default-value omission gives forward-compat: additive-only schema changes don't shift bytes for old signers that leave new fields unset.

Rollout

The wire-compat layer keeps historical replay safe, but live partial-rollout still requires coordination because the new envelope is wire-incompatible with the old shape:

  1. Disable API repo from issuing CreateReward / DeleteReward (the only legitimate producer).
  2. Drain in-flight reward txs from mempools.
  3. Roll the validator fleet to this binary (and Add reward_pools schema and migrate core_rewards #222) one validator at a time.
  4. Re-enable API once all validators are upgraded; API now emits the new envelope + rewards_manager_pubkey.

Reads (GetRewardAttestation, GetReward, GetRewards, GetRewardPool) are non-consensus and stay safe across versions.

PR 3 (#228) adds the per-RM gate to GetRewardSenderAttestation and GetDeleteRewardSenderAttestation, completing the Solana-side rotation flow.

Test plan

  • go build ./... clean.
  • go vet ./pkg/common/... ./pkg/core/server/... ./pkg/sdk/... ./pkg/integration_tests/... — no new warnings.
  • go test ./pkg/common/... ./pkg/core/server/ ./pkg/rewards/... passes:
    • signing roundtrip per body type, oneof discrimination, tamper-breaks-signature
    • legacy proto roundtrip via preserved unknown fields
    • live-validation rejects legacy bytes
  • Integration test (pkg/integration_tests/13_reward_pools_test.go):
    • non-pubkey rewards_manager_pubkey rejected at CreateRewardPool
    • signer not in initial authorities → reject
    • duplicate RM pubkey → reject
    • pool member can CreateReward; non-member cannot
    • CreateReward without rewards_manager_pubkey → reject
    • SetRewardPoolAuthorities by non-member → reject
    • empty authorities → reject; non-eth-address authorities → reject
    • rotation removes signer; rotated-out signer can no longer create rewards
    • GetRewards finds pool-attached rewards via checksum-case address

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR is the second step in the reward-authority rotation work: it adds first-class reward-pool transactions to the CometBFT wire format and switches reward signing to a protobuf body+signature envelope. It also updates the SDK/tests/examples to use the new signing shape and introduces pool-based gating for CreateReward.

Changes:

  • Add RewardPoolMessage / RewardPoolBody plus CreateRewardPool and SetRewardPoolAuthorities transaction types.
  • Refactor reward signing and verification from ad-hoc canonical payloads to deterministic protobuf ProtoSign / ProtoRecover.
  • Gate CreateReward on reward-pool membership when pool_address is provided, and update SDK/tests/examples for the new API shape.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
proto/core/v1/types.proto Defines new reward/reward-pool wire messages and moves deadline/signature into signed bodies/envelopes.
pkg/sdk/rewards/rewards.go Updates SDK reward send paths to sign protobuf bodies and adds reward-pool helpers.
pkg/integration_tests/12_rewards_test.go Adjusts reward lifecycle tests to the new SDK method signatures.
pkg/core/server/rewards.go Reworks reward validation/finalization around body+signature envelopes and pool-based gating.
pkg/core/server/reward_pools.go Adds server-side validation/finalization for reward-pool transactions.
pkg/core/server/abci.go Wires reward-pool tx validation and finalization into ABCI processing.
pkg/core/db/writes.sql.go Generates DB write helpers for inserting/updating reward pools.
pkg/core/db/sql/writes.sql Adds SQL for first-class reward-pool inserts and authority updates.
pkg/core/db/sql/reads.sql Adds SQL for loading reward pools and querying them by authority.
pkg/core/db/reads.sql.go Generates DB read helpers for reward-pool queries.
pkg/common/reward_signing.go Removes the old reward-specific deterministic signing helpers.
pkg/common/proto.go Makes protobuf signing/recovery deterministic and body-oriented.
pkg/common/proto_test.go Adds tests for protobuf signing/recovery across reward and reward-pool bodies.
pkg/api/core/v1/types.pb.go Regenerated protobuf Go types for the new reward/reward-pool schema.
examples/rewards/main.go Updates the rewards example to the new SDK method signature.

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

Comment thread pkg/core/server/rewards.go Outdated
Comment thread pkg/core/server/rewards.go Outdated
Comment thread pkg/core/server/rewards.go Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/sdk/rewards/rewards.go
Comment thread pkg/core/db/sql/writes.sql Outdated
Comment thread pkg/core/server/reward_pools.go

@raymondjacobson raymondjacobson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Before sending the first tx, do we need to get full adoption? What happens if 2/3rds of nodes have this change and we send the tx type?

rickyrombo added a commit that referenced this pull request May 5, 2026
… addresses

Consensus correctness:
- finalizeCreateReward, finalizeDeleteReward, finalizeSetRewardPoolAuthorities
  now re-check authorization against post-prior-tx state via s.getDb(). A
  sibling tx earlier in the same block can rotate the signer out between
  validate (pre-block state) and finalize.
- Extract checkPoolAuthorization helper used by both validate and finalize.

Error handling:
- validateCreateReward distinguishes pgx.ErrNoRows from transient DB errors
  (matches validateSetRewardPoolAuthorities). Transient blips no longer
  reject valid txs as 'pool not found'.

API ergonomics:
- CreateRewardPool takes a caller-chosen pool_address (≤64 chars, unique).
  Caller knows the address up front and can compose dependent
  CreateReward / SetRewardPoolAuthorities calls without round-tripping.
  Replaces the txhash-derived address scheme.
- Add GetRewardPool RPC + SDK method for read-side resolution.

Tests:
- New pkg/integration_tests/13_reward_pools_test.go covers create-pool,
  pool-gated CreateReward, rotation via SetRewardPoolAuthorities,
  rotated-out signer rejection, duplicate-address rejection, and
  empty-authorities rejection.
- proto_test.go RewardPoolBody_Create case includes pool_address.

Misc:
- Fix stale UpdateRewardPoolAuthorities comment in writes.sql that still
  referenced the dropped Add/RemoveRewardPoolAuthority pair.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo requested a review from Copilot May 5, 2026 23:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 19 out of 20 changed files in this pull request and generated 5 comments.


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

Comment thread proto/core/v1/types.proto
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/core/server/reward_pools.go
Comment thread proto/core/v1/types.proto Outdated
rickyrombo added a commit that referenced this pull request May 5, 2026
Wire-compat (block-sync-from-genesis):
- Add LegacyRewardMessage / LegacyCreateReward / LegacyDeleteReward proto
  types pinning the pre-pool-rollout wire shape (oneof at tags 1000/1001
  with deadline + signature embedded in each action). DO NOT REMOVE.
- Add legacy signer recovery (pkg/common/legacy_reward_signing.go) using
  the original sha256-over-pipe-delimited-canonical-string scheme.
- Add tryParseLegacyReward: when the new RewardMessage decodes with
  Body == nil, re-marshal preserves the unknown tag-1000/1001 bytes;
  decode them as LegacyRewardMessage. Translate to the synthetic-pool
  finalize path that already handles "no pool_address" CreateReward txs.
- Hook into isValidRewardTransaction / finalizeRewards. Live traffic is
  unaffected (the SDK only emits the new envelope); this path activates
  during historical replay.

Authority validation:
- validateAuthorityList rejects non-eth-address strings in both
  CreateRewardPool and SetRewardPoolAuthorities. Without this, a current
  authority could rotate a pool to ["not-an-address"], permanently
  orphaning every attached reward (the bogus address can never satisfy
  signer recovery).

GetRewards case-sensitivity fix:
- Stored authorities are lowercased by rewards.CanonicalAuthorities, but
  GetRewardsByClaimAuthority does case-sensitive @> array containment.
  Lowercase the caller-supplied claim_authority at the connect.go
  handler so checksum-case addresses (returned by common.PrivKeyToAddress)
  resolve correctly.

Tests:
- pkg/core/server/rewards_legacy_test.go: end-to-end roundtrip — encode
  legacy bytes, decode under new shape with Body == nil, re-extract
  via tryParseLegacyReward, recover signer with legacy scheme.
- 13_reward_pools_test.go: assert pool-attached reward is visible via
  GetRewards using checksum-case address.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo requested a review from Copilot May 5, 2026 23:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 22 out of 23 changed files in this pull request and generated 8 comments.


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

Comment thread proto/core/v1/types.proto
Comment thread pkg/sdk/rewards/rewards.go
Comment thread pkg/sdk/rewards/rewards.go
Comment thread proto/core/v1/types.proto Outdated
Comment thread pkg/integration_tests/13_reward_pools_test.go Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/core/server/rewards.go Outdated
Comment thread pkg/common/proto.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 22 out of 23 changed files in this pull request and generated 5 comments.


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

Comment thread pkg/core/server/rewards.go Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/core/server/connect.go Outdated
Comment thread pkg/core/db/writes.sql.go Outdated
Comment thread pkg/core/db/writes.sql.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 19 changed files in this pull request and generated 6 comments.


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

Comment thread pkg/core/server/reward_pools.go
Comment thread pkg/core/server/connect.go
Comment thread pkg/sdk/rewards/rewards.go Outdated
Comment thread examples/rewards/main.go Outdated
Comment thread pkg/core/server/rewards.go Outdated
Comment thread pkg/core/server/rewards_legacy.go Outdated
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-schema branch from 70097a6 to 7ac5c4f Compare May 6, 2026 23:00
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from b9d1b34 to 3669e95 Compare May 7, 2026 00:26
rickyrombo added a commit that referenced this pull request May 7, 2026
Wires up the rotation flow on top of PR1 (#222) and PR2 (#225). Once
this lands, an OAP authority change for a launchpad coin's pool can be
followed by validator-signed Solana CreateSenderPublic /
DeleteSenderPublic instructions to bring the on-chain reward manager
into agreement.

## senderGateForRM dispatch

Single helper that resolves which gating regime applies to a given RM:
  - If a row in core_reward_pools matches the RM, the pool is the source
    of truth: addAttestation iff addr ∈ pool.authorities;
    deleteAttestation iff addr ∉ pool.authorities.
  - Otherwise, fall through to the legacy validator/AAO trust set
    (gatherEligibleSenderAddresses).

Transient DB errors are propagated rather than silently downgrading to
the validator/AAO path — a temporary blip should not weaken the gate.

## GetRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∈ pool.authorities for that
RM. Non-pool RMs (notably AUDIO): existing validator/AAO check.

## GetDeleteRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∉ pool.authorities. This is
the rotation-out signal — once OAP rotates a key out of its pool, the
validator can be asked to sign a Solana DeleteSenderPublic attestation,
preventing the rotated-out key from continuing to attest claims on
Solana.

Non-pool RMs: existing "must NOT be a validator/AAO" check.

## GetRewardAttestation restored

Removes the temporary kill-switch from #215. The handler is the original
implementation, but the authority check is now pool-gated for free:
dbReward.ClaimAuthorities is sourced from core_reward_pools.authorities
via the LEFT JOIN added in PR1, so rotating an authority out via
SetRewardPoolAuthorities immediately revokes their ability to
authenticate claim attestations.

## AUDIO RM denylist

config.AudioRewardsManagerPubkey() returns the AUDIO RM pubkey for the
current runtime environment (per-env constants in pkg/core/config/rewards.go,
empty by default — to be filled in for staging/prod before merge).
validateRewardsManagerPubkey refuses CreateRewardPool that targets the
configured AUDIO RM. Without this, an attacker could create a pool for
the AUDIO RM with their own keys as initial authorities and have
validators sign AUDIO sender attestations on their behalf.

## SDK helpers

Adds Rewards.GetRewardSenderAttestation and
Rewards.GetDeleteRewardSenderAttestation so rotation tooling (or the API
repo) can drive Solana sender registration / deregistration without
constructing connect requests by hand.

## Tests

- pkg/core/server/reward_pools_test.go: unit tests for
  validateRewardsManagerPubkey covering shape rejections (empty,
  whitespace, mig_ prefix, non-base58, wrong length) plus AUDIO denylist.
- pkg/integration_tests/13_reward_pools_test.go: extended with the
  full rotation flow against the validator endpoints — sign create for
  current authority, refuse create for rotated-out, sign delete for
  rotated-out, refuse delete for current authority.
- pkg/integration_tests/12_rewards_test.go: removes the temporary
  artist-coin-attestation skip markers now that GetRewardAttestation is
  restored.

## TODO before merge

- Fill in DevAudioRewardsManagerPubkey / StageAudioRewardsManagerPubkey
  / ProdAudioRewardsManagerPubkey in pkg/core/config/rewards.go.
- After user provides claim_authorities → RM pubkey mapping for the
  existing reward rows: update PR1's backfill (00033 migration) to use
  real RM pubkeys instead of mig_<md5> synthetic identifiers, and drop
  the synthetic-pool fallback from PR2's wire-compat layer.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-schema branch from 7ac5c4f to 0ddace0 Compare May 7, 2026 00:49
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from 3669e95 to da41064 Compare May 7, 2026 00:50
rickyrombo added a commit that referenced this pull request May 7, 2026
Wires up the rotation flow on top of PR1 (#222) and PR2 (#225). Once
this lands, an OAP authority change for a launchpad coin's pool can be
followed by validator-signed Solana CreateSenderPublic /
DeleteSenderPublic instructions to bring the on-chain reward manager
into agreement.

## senderGateForRM dispatch

Single helper that resolves which gating regime applies to a given RM:
  - If a row in core_reward_pools matches the RM, the pool is the source
    of truth: addAttestation iff addr ∈ pool.authorities;
    deleteAttestation iff addr ∉ pool.authorities.
  - Otherwise, fall through to the legacy validator/AAO trust set
    (gatherEligibleSenderAddresses).

Transient DB errors are propagated rather than silently downgrading to
the validator/AAO path — a temporary blip should not weaken the gate.

## GetRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∈ pool.authorities for that
RM. Non-pool RMs (notably AUDIO): existing validator/AAO check.

## GetDeleteRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∉ pool.authorities. This is
the rotation-out signal — once OAP rotates a key out of its pool, the
validator can be asked to sign a Solana DeleteSenderPublic attestation,
preventing the rotated-out key from continuing to attest claims on
Solana.

Non-pool RMs: existing "must NOT be a validator/AAO" check.

## GetRewardAttestation restored

Removes the temporary kill-switch from #215. The handler is the original
implementation, but the authority check is now pool-gated for free:
dbReward.ClaimAuthorities is sourced from core_reward_pools.authorities
via the LEFT JOIN added in PR1, so rotating an authority out via
SetRewardPoolAuthorities immediately revokes their ability to
authenticate claim attestations.

## AUDIO RM denylist

config.AudioRewardsManagerPubkey() returns the AUDIO RM pubkey for the
current runtime environment (per-env constants in pkg/core/config/rewards.go,
empty by default — to be filled in for staging/prod before merge).
validateRewardsManagerPubkey refuses CreateRewardPool that targets the
configured AUDIO RM. Without this, an attacker could create a pool for
the AUDIO RM with their own keys as initial authorities and have
validators sign AUDIO sender attestations on their behalf.

## SDK helpers

Adds Rewards.GetRewardSenderAttestation and
Rewards.GetDeleteRewardSenderAttestation so rotation tooling (or the API
repo) can drive Solana sender registration / deregistration without
constructing connect requests by hand.

## Tests

- pkg/core/server/reward_pools_test.go: unit tests for
  validateRewardsManagerPubkey covering shape rejections (empty,
  whitespace, mig_ prefix, non-base58, wrong length) plus AUDIO denylist.
- pkg/integration_tests/13_reward_pools_test.go: extended with the
  full rotation flow against the validator endpoints — sign create for
  current authority, refuse create for rotated-out, sign delete for
  rotated-out, refuse delete for current authority.
- pkg/integration_tests/12_rewards_test.go: removes the temporary
  artist-coin-attestation skip markers now that GetRewardAttestation is
  restored.

## TODO before merge

- Fill in DevAudioRewardsManagerPubkey / StageAudioRewardsManagerPubkey
  / ProdAudioRewardsManagerPubkey in pkg/core/config/rewards.go.
- After user provides claim_authorities → RM pubkey mapping for the
  existing reward rows: update PR1's backfill (00033 migration) to use
  real RM pubkeys instead of mig_<md5> synthetic identifiers, and drop
  the synthetic-pool fallback from PR2's wire-compat layer.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from da41064 to ea6e545 Compare May 7, 2026 00:54
rickyrombo added a commit that referenced this pull request May 7, 2026
Wires up the rotation flow on top of PR1 (#222) and PR2 (#225). Once
this lands, an OAP authority change for a launchpad coin's pool can be
followed by validator-signed Solana CreateSenderPublic /
DeleteSenderPublic instructions to bring the on-chain reward manager
into agreement.

## senderGateForRM dispatch

Single helper that resolves which gating regime applies to a given RM:
  - If a row in core_reward_pools matches the RM, the pool is the source
    of truth: addAttestation iff addr ∈ pool.authorities;
    deleteAttestation iff addr ∉ pool.authorities.
  - Otherwise, fall through to the legacy validator/AAO trust set
    (gatherEligibleSenderAddresses).

Transient DB errors are propagated rather than silently downgrading to
the validator/AAO path — a temporary blip should not weaken the gate.

## GetRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∈ pool.authorities for that
RM. Non-pool RMs (notably AUDIO): existing validator/AAO check.

## GetDeleteRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∉ pool.authorities. This is
the rotation-out signal — once OAP rotates a key out of its pool, the
validator can be asked to sign a Solana DeleteSenderPublic attestation,
preventing the rotated-out key from continuing to attest claims on
Solana.

Non-pool RMs: existing "must NOT be a validator/AAO" check.

## GetRewardAttestation restored

Removes the temporary kill-switch from #215. The handler is the original
implementation, but the authority check is now pool-gated for free:
dbReward.ClaimAuthorities is sourced from core_reward_pools.authorities
via the LEFT JOIN added in PR1, so rotating an authority out via
SetRewardPoolAuthorities immediately revokes their ability to
authenticate claim attestations.

## AUDIO RM denylist

config.AudioRewardsManagerPubkey() returns the AUDIO RM pubkey for the
current runtime environment (per-env constants in pkg/core/config/rewards.go,
empty by default — to be filled in for staging/prod before merge).
validateRewardsManagerPubkey refuses CreateRewardPool that targets the
configured AUDIO RM. Without this, an attacker could create a pool for
the AUDIO RM with their own keys as initial authorities and have
validators sign AUDIO sender attestations on their behalf.

## SDK helpers

Adds Rewards.GetRewardSenderAttestation and
Rewards.GetDeleteRewardSenderAttestation so rotation tooling (or the API
repo) can drive Solana sender registration / deregistration without
constructing connect requests by hand.

## Tests

- pkg/core/server/reward_pools_test.go: unit tests for
  validateRewardsManagerPubkey covering shape rejections (empty,
  whitespace, mig_ prefix, non-base58, wrong length) plus AUDIO denylist.
- pkg/integration_tests/13_reward_pools_test.go: extended with the
  full rotation flow against the validator endpoints — sign create for
  current authority, refuse create for rotated-out, sign delete for
  rotated-out, refuse delete for current authority.
- pkg/integration_tests/12_rewards_test.go: removes the temporary
  artist-coin-attestation skip markers now that GetRewardAttestation is
  restored.

## TODO before merge

- Fill in DevAudioRewardsManagerPubkey / StageAudioRewardsManagerPubkey
  / ProdAudioRewardsManagerPubkey in pkg/core/config/rewards.go.
- After user provides claim_authorities → RM pubkey mapping for the
  existing reward rows: update PR1's backfill (00033 migration) to use
  real RM pubkeys instead of mig_<md5> synthetic identifiers, and drop
  the synthetic-pool fallback from PR2's wire-compat layer.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-schema branch from 0ddace0 to dc90e27 Compare May 7, 2026 02:44
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from 816f923 to 63d60c9 Compare May 7, 2026 02:52
@rickyrombo
rickyrombo requested a review from Copilot May 7, 2026 02:52
rickyrombo added a commit that referenced this pull request May 7, 2026
Wires up the rotation flow on top of PR1 (#222) and PR2 (#225). Once
this lands, an OAP authority change for a launchpad coin's pool can be
followed by validator-signed Solana CreateSenderPublic /
DeleteSenderPublic instructions to bring the on-chain reward manager
into agreement.

## senderGateForRM dispatch

Single helper that resolves which gating regime applies to a given RM:
  - If a row in core_reward_pools matches the RM, the pool is the source
    of truth: addAttestation iff addr ∈ pool.authorities;
    deleteAttestation iff addr ∉ pool.authorities.
  - Otherwise, fall through to the legacy validator/AAO trust set
    (gatherEligibleSenderAddresses).

Transient DB errors are propagated rather than silently downgrading to
the validator/AAO path — a temporary blip should not weaken the gate.

## GetRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∈ pool.authorities for that
RM. Non-pool RMs (notably AUDIO): existing validator/AAO check.

## GetDeleteRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∉ pool.authorities. This is
the rotation-out signal — once OAP rotates a key out of its pool, the
validator can be asked to sign a Solana DeleteSenderPublic attestation,
preventing the rotated-out key from continuing to attest claims on
Solana.

Non-pool RMs: existing "must NOT be a validator/AAO" check.

## GetRewardAttestation restored

Removes the temporary kill-switch from #215. The handler is the original
implementation, but the authority check is now pool-gated for free:
dbReward.ClaimAuthorities is sourced from core_reward_pools.authorities
via the LEFT JOIN added in PR1, so rotating an authority out via
SetRewardPoolAuthorities immediately revokes their ability to
authenticate claim attestations.

## AUDIO RM denylist

config.AudioRewardsManagerPubkey() returns the AUDIO RM pubkey for the
current runtime environment (per-env constants in pkg/core/config/rewards.go,
empty by default — to be filled in for staging/prod before merge).
validateRewardsManagerPubkey refuses CreateRewardPool that targets the
configured AUDIO RM. Without this, an attacker could create a pool for
the AUDIO RM with their own keys as initial authorities and have
validators sign AUDIO sender attestations on their behalf.

## SDK helpers

Adds Rewards.GetRewardSenderAttestation and
Rewards.GetDeleteRewardSenderAttestation so rotation tooling (or the API
repo) can drive Solana sender registration / deregistration without
constructing connect requests by hand.

## Tests

- pkg/core/server/reward_pools_test.go: unit tests for
  validateRewardsManagerPubkey covering shape rejections (empty,
  whitespace, mig_ prefix, non-base58, wrong length) plus AUDIO denylist.
- pkg/integration_tests/13_reward_pools_test.go: extended with the
  full rotation flow against the validator endpoints — sign create for
  current authority, refuse create for rotated-out, sign delete for
  rotated-out, refuse delete for current authority.
- pkg/integration_tests/12_rewards_test.go: removes the temporary
  artist-coin-attestation skip markers now that GetRewardAttestation is
  restored.

## TODO before merge

- Fill in DevAudioRewardsManagerPubkey / StageAudioRewardsManagerPubkey
  / ProdAudioRewardsManagerPubkey in pkg/core/config/rewards.go.
- After user provides claim_authorities → RM pubkey mapping for the
  existing reward rows: update PR1's backfill (00033 migration) to use
  real RM pubkeys instead of mig_<md5> synthetic identifiers, and drop
  the synthetic-pool fallback from PR2's wire-compat layer.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 19 changed files in this pull request and generated 1 comment.

Comment thread pkg/core/server/connect.go Outdated
rickyrombo added a commit that referenced this pull request May 7, 2026
The launchpad-era CreateReward / DeleteReward proto shapes (RewardMessage
oneof at tags 1000/1001 with deadline + signature embedded inside each
action) are pre-pool-rollout artifacts. They were preserved in PR #225's
wire-compat layer so block-sync-from-genesis could replay historical
reward txs without diverging from the migration's apphash.

We're planning a network restart with genesis replay anyway, so the new
chain will never contain those legacy bytes. The wire-compat path
becomes dead code after the restart, and this PR removes it.

Removed:
- LegacyRewardMessage / LegacyCreateReward / LegacyDeleteReward proto
  types.
- pkg/common/legacy_reward_signing.go (sha256-over-canonical-string
  legacy signing scheme).
- tryParseLegacyReward + the Body == nil dispatch branches in
  isValidRewardTransaction and finalizeRewards.
- pkg/core/server/rewards_legacy.go and rewards_legacy_test.go
  (~200 lines).
- GetLaunchpadRMByAuthority SQL query.
- UpsertSyntheticRewardPool SQL query.
- MigratedPoolAddress helper + tests.

Schema cleanup via new migration 00034:
- launchpad_authority_rm table dropped via 00034. PR1's 00033 stays
  unchanged on disk: existing chains have already applied it, and a
  modify-in-place would diverge between the on-disk version and
  what's recorded in their migration history. 00034 brings the
  schema in line with the code's expectations on both fresh-genesis
  and post-restart state.

After this PR:
- isValidRewardTransaction / finalizeRewards return "reward message
  body is nil" for any envelope with Body == nil. No special-casing.
- validateRewardsManagerPubkey is just shape validation (base58 32
  bytes) plus the AUDIO denylist.

Sequencing: this PR ships AFTER #222, #225, #228 merge AND the network
restart with genesis replay. Until that restart, the wire-compat layer
remains needed to keep historical replay deterministic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Introduces a `core_reward_pools` table that owns the set of eth addresses
authorized to attest for a reward, replacing the inline
`core_rewards.claim_authorities` array. Pools are first-class so authority
sets can be rotated without rewriting every reward row.

This is the first of three PRs in the rotation initiative:
1. (this PR) Schema + data migration.
2. CometBFT transactions for managing pools (`CreateRewardPool`,
   `AddRewardPoolAuthority`, `RemoveRewardPoolAuthority`).
3. Validator endpoint cutover (`GetRewardAttestation`,
   `GetRewardSenderAttestation`, `GetDeleteRewardSenderAttestation`)
   to gate on current pool authorities instead of the row-frozen list.

Schema (00033_reward_pools.sql):

  create table core_reward_pools (
      address     text primary key,
      authorities text[] not null default '{}',
      created_at  timestamp with time zone default now(),
      updated_at  timestamp with time zone default now()
  );
  create index idx_core_reward_pools_authorities
      on core_reward_pools using gin (authorities);

  alter table core_rewards add column pool_address text;
  alter table core_rewards add constraint fk_core_rewards_pool_address
      foreign key (pool_address) references core_reward_pools(address)
      on delete cascade;
  -- backfill, then:
  drop index idx_core_rewards_claim_authorities;
  alter table core_rewards drop column claim_authorities;

Backfill groups every existing `core_rewards` row by its canonical
(trim/lower/dedup/sort) `claim_authorities` set; inserts one synthetic
pool per unique set with `address = 'mig_' || md5(comma-joined canonical)`;
populates each row's `pool_address` accordingly. The down migration
restores `claim_authorities` from `pool.authorities` before tearing down.

SQL queries (reads.sql, writes.sql):
- All reward `SELECT`s now `LEFT JOIN core_reward_pools` and alias
  `coalesce(p.authorities, '{}')::text[]` as `claim_authorities`, so
  callers see the same field but it's sourced from the pool.
- `GetRewardsByClaimAuthority` uses `p.authorities @> array[$1::text]`
  so the gin index on authorities is actually used (= ANY can't).
- `InsertCoreReward` / `UpdateCoreReward` write `pool_address` instead
  of `claim_authorities`.
- New `UpsertSyntheticRewardPool` uses `DO UPDATE SET authorities = excluded.authorities`
  — self-correcting if canonicalization ever drifts (the address is
  derived from the authorities, so any drift would otherwise silently
  preserve a stale row).

Handlers:
- `pkg/rewards/reward_pool.go` (new): `CanonicalAuthorities`
  (trim, lower, dedup, sort) and `MigratedPoolAddress` helpers, with
  the same canonicalization rules as the SQL backfill so Go-derived
  and SQL-derived synthetic pool addresses are guaranteed to match.
  Pinned by table-driven tests + golden-vector tests in
  `reward_pool_test.go`.
- `pkg/core/server/rewards.go`: `finalizeCreateReward` derives the
  synthetic pool address from the message's inline `claim_authorities`,
  upserts the pool row, and inserts the reward with `pool_address`
  populated. The on-chain `CreateReward` proto is unchanged in this
  PR; PR 2 introduces `pool_address` as a first-class field.
- `pkg/core/server/state_sync.go`: adds `core_reward_pools` to the
  table dump list so state-synced nodes inherit the new table.

Test plan:
- [x] `go build ./...` clean.
- [x] `go vet ./pkg/core/server/... ./pkg/rewards/...` no new warnings.
- [x] `go test ./pkg/rewards/...` passes; vectors verified against
      shell md5sum to match what the SQL backfill produces.
- [ ] Apply against a snapshot of staging; confirm one pool per unique
      authority set and every reward row's `pool_address` populated.
- [ ] Apply down migration; confirm `claim_authorities` restored.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from 63d60c9 to 9d1f73d Compare May 7, 2026 05:48
rickyrombo added a commit that referenced this pull request May 7, 2026
Wires up the rotation flow on top of PR1 (#222) and PR2 (#225). Once
this lands, an OAP authority change for a launchpad coin's pool can be
followed by validator-signed Solana CreateSenderPublic /
DeleteSenderPublic instructions to bring the on-chain reward manager
into agreement.

## senderGateForRM dispatch

Single helper that resolves which gating regime applies to a given RM:
  - If a row in core_reward_pools matches the RM, the pool is the source
    of truth: addAttestation iff addr ∈ pool.authorities;
    deleteAttestation iff addr ∉ pool.authorities.
  - Otherwise, fall through to the legacy validator/AAO trust set
    (gatherEligibleSenderAddresses).

Transient DB errors are propagated rather than silently downgrading to
the validator/AAO path — a temporary blip should not weaken the gate.

## GetRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∈ pool.authorities for that
RM. Non-pool RMs (notably AUDIO): existing validator/AAO check.

## GetDeleteRewardSenderAttestation

Pool-managed RMs: sign iff requested address ∉ pool.authorities. This is
the rotation-out signal — once OAP rotates a key out of its pool, the
validator can be asked to sign a Solana DeleteSenderPublic attestation,
preventing the rotated-out key from continuing to attest claims on
Solana.

Non-pool RMs: existing "must NOT be a validator/AAO" check.

## GetRewardAttestation restored

Removes the temporary kill-switch from #215. The handler is the original
implementation, but the authority check is now pool-gated for free:
dbReward.ClaimAuthorities is sourced from core_reward_pools.authorities
via the LEFT JOIN added in PR1, so rotating an authority out via
SetRewardPoolAuthorities immediately revokes their ability to
authenticate claim attestations.

## AUDIO RM denylist

config.AudioRewardsManagerPubkey() returns the AUDIO RM pubkey for the
current runtime environment (per-env constants in pkg/core/config/rewards.go,
empty by default — to be filled in for staging/prod before merge).
validateRewardsManagerPubkey refuses CreateRewardPool that targets the
configured AUDIO RM. Without this, an attacker could create a pool for
the AUDIO RM with their own keys as initial authorities and have
validators sign AUDIO sender attestations on their behalf.

## SDK helpers

Adds Rewards.GetRewardSenderAttestation and
Rewards.GetDeleteRewardSenderAttestation so rotation tooling (or the API
repo) can drive Solana sender registration / deregistration without
constructing connect requests by hand.

## Tests

- pkg/core/server/reward_pools_test.go: unit tests for
  validateRewardsManagerPubkey covering shape rejections (empty,
  whitespace, mig_ prefix, non-base58, wrong length) plus AUDIO denylist.
- pkg/integration_tests/13_reward_pools_test.go: extended with the
  full rotation flow against the validator endpoints — sign create for
  current authority, refuse create for rotated-out, sign delete for
  rotated-out, refuse delete for current authority.
- pkg/integration_tests/12_rewards_test.go: removes the temporary
  artist-coin-attestation skip markers now that GetRewardAttestation is
  restored.

## TODO before merge

- Fill in DevAudioRewardsManagerPubkey / StageAudioRewardsManagerPubkey
  / ProdAudioRewardsManagerPubkey in pkg/core/config/rewards.go.
- After user provides claim_authorities → RM pubkey mapping for the
  existing reward rows: update PR1's backfill (00033 migration) to use
  real RM pubkeys instead of mig_<md5> synthetic identifiers, and drop
  the synthetic-pool fallback from PR2's wire-compat layer.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo requested a review from Copilot May 7, 2026 18:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 19 changed files in this pull request and generated 2 comments.

Comment thread pkg/core/server/connect.go
Comment thread pkg/core/server/reward_pools.go Outdated
rickyrombo added a commit that referenced this pull request May 8, 2026
The launchpad-era CreateReward / DeleteReward proto shapes (RewardMessage
oneof at tags 1000/1001 with deadline + signature embedded inside each
action) are pre-pool-rollout artifacts. They were preserved in PR #225's
wire-compat layer so block-sync-from-genesis could replay historical
reward txs without diverging from the migration's apphash.

We're planning a network restart with genesis replay anyway, so the new
chain will never contain those legacy bytes. The wire-compat path
becomes dead code after the restart, and this PR removes it.

Removed:
- LegacyRewardMessage / LegacyCreateReward / LegacyDeleteReward proto
  types.
- pkg/common/legacy_reward_signing.go (sha256-over-canonical-string
  legacy signing scheme).
- tryParseLegacyReward + the Body == nil dispatch branches in
  isValidRewardTransaction and finalizeRewards.
- pkg/core/server/rewards_legacy.go and rewards_legacy_test.go
  (~200 lines).
- GetLaunchpadRMByAuthority SQL query.
- UpsertSyntheticRewardPool SQL query.
- MigratedPoolAddress helper + tests.

Schema cleanup via new migration 00034:
- launchpad_authority_rm table dropped via 00034. PR1's 00033 stays
  unchanged on disk: existing chains have already applied it, and a
  modify-in-place would diverge between the on-disk version and
  what's recorded in their migration history. 00034 brings the
  schema in line with the code's expectations on both fresh-genesis
  and post-restart state.

After this PR:
- isValidRewardTransaction / finalizeRewards return "reward message
  body is nil" for any envelope with Body == nil. No special-casing.
- validateRewardsManagerPubkey is just shape validation (base58 32
  bytes) plus the AUDIO denylist.

Sequencing: this PR ships AFTER #222, #225, #228 merge AND the network
restart with genesis replay. Until that restart, the wire-compat layer
remains needed to keep historical replay deterministic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-schema branch from de3938f to 7ca1156 Compare May 8, 2026 01:00
Adds two cometbft transactions for managing the per-RM reward pool
primitive introduced in PR1 (#222), and gates CreateReward on pool
membership. Stacked on PR1.

- CreateRewardPool { rewards_manager_pubkey, authorities[] }
- SetRewardPoolAuthorities { rewards_manager_pubkey, authorities[] }

The pool's identity IS the Solana reward manager pubkey — there is no
separate "pool address" concept. This makes the pool↔RM binding
unforgeable by construction and sets up PR3's per-RM sender-attestation
gate to be a trivial lookup.

rewards_manager_pubkey is validated as base58 32 bytes; the 'mig_'
prefix used by PR1's synthetic-pool backfill is rejected for first-class
CreateRewardPool. Synthetic pools never gate sender attestations under
PR3 because they don't decode as real RM pubkeys.

CreateReward { reward_id, name, amount, rewards_manager_pubkey }
requires an existing first-class pool. The recovered signer must be a
current member of pool.authorities (re-checked at finalize time, since
block ordering can rotate the signer out between validate and finalize).
Inline claim_authorities is dropped (tag 4 reserved); use a pool.

Replaces the per-action custom canonical signing scheme with a cosmos-
style { body, signature } envelope. body holds (deadline_block_height,
oneof action), and ProtoSign / ProtoRecover use proto.MarshalOptions
{Deterministic: true} over the body bytes. Cross-action replay is
prevented by the body's oneof field tag being part of signed bytes.

The body+signature envelope is wire-incompatible with the pre-pool
network's RewardMessage shape. To keep block-sync-from-genesis working,
LegacyRewardMessage / LegacyCreateReward / LegacyDeleteReward proto
types preserve the old wire format, and tryParseLegacyReward in
rewards_legacy.go recovers them from preserved unknown fields. The
legacy signing scheme (sha256 over canonical pipe-delimited string) is
ported as common.Legacy*RewardData / Legacy*Recover* helpers.

Asymmetric gate: legacy bytes are REJECTED at validate-time
(CheckTx + ProcessProposal) because legacy CreateReward was permission-
less by design — accepting them live would reopen the exact exploit
class this PR closes (attacker crafts legacy bytes with arbitrary
inline claim_authorities, bypasses the pool gate). Legacy bytes are
ACCEPTED at finalize-time (FinalizeBlock) because block-sync only
invokes that path and historical blocks were already validated by the
old network.

finalizeCreateReward, finalizeDeleteReward, finalizeSetRewardPool-
Authorities all re-check authorization against post-prior-tx state via
s.getDb(). validateBlockTxs runs against pre-block state, so an
earlier tx in the same block can rotate the signer out before a later
one runs.

- validateAuthorityList: rejects non-eth-address strings on
  CreateRewardPool / SetRewardPoolAuthorities (would otherwise let a
  current authority orphan the pool by rotating to ["not-an-address"]).
- GetRewards lowercases the caller-supplied claim_authority to match
  the canonicalized stored values (CanonicalAuthorities lowercases;
  the underlying GetRewardsByClaimAuthority uses case-sensitive @>).

- pkg/common/proto_test.go: signing roundtrip, oneof discrimination,
  tampering breaks signature.
- pkg/core/server/rewards_legacy_test.go: legacy bytes round-trip
  through unknown-field preservation; live-validation rejects legacy.
- pkg/integration_tests/13_reward_pools_test.go: create-pool, pool-gated
  CreateReward, rotation via SetRewardPoolAuthorities, post-rotation
  signer rejection, duplicate / non-pubkey / 'mig_'-prefix rejections,
  GetRewards finds pool-attached rewards via checksum-case address.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from 9d1f73d to 2f5a5f0 Compare May 8, 2026 01:04
# Conflicts:
#	pkg/core/server/rewards.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 19 changed files in this pull request and generated 3 comments.

Comment thread proto/core/v1/types.proto Outdated
Comment thread pkg/core/server/reward_pools.go
Comment thread pkg/core/server/rewards_legacy.go
Three review-driven fixes:

1. Move CreateReward.rewards_manager_pubkey to tag 7; reserve tags 4-6
   (previously claim_authorities, deadline_block_height, signature on
   the pre-pool shape). Same for DeleteReward: reserve tags 2-3 and
   keep address at tag 1. The wire-compat layer routes legacy bytes
   through Legacy{Create,Delete}Reward so the live decoders never see
   old data, but reserving makes that invariant explicit and keeps
   future tools / refactors from silently misinterpreting legacy bytes.

2. finalizeSetRewardPoolAuthorities now re-runs validateRewardsManagerPubkey
   and validateAuthorityList, mirroring the defense-in-depth check
   already in finalizeCreateRewardPool. Block-sync replay invokes
   FinalizeBlock without ProcessProposal/CheckTx, so without the
   re-validation a malformed message that ever made it onto the chain
   could orphan a pool by writing it to an empty/invalid authority set.

3. finalizeLegacyDeleteReward now distinguishes pgx.ErrNoRows from
   transient DB errors, mirroring the non-legacy finalizeDeleteReward.
   "Reward not found" is a validation error; anything else is internal.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 18 out of 19 changed files in this pull request and generated 2 comments.

Comment thread proto/core/v1/types.proto
// chicken-and-egg.
message RewardMessage {
RewardBody body = 1;
string signature = 2; // signature over the deterministic marshaling of body
Comment on lines +153 to +157
// Upsert the pool so any subsequent legacy replays of rewards
// targeting the same RM converge on the same authority set.
if err := qtx.UpsertSyntheticRewardPool(ctx, db.UpsertSyntheticRewardPoolParams{
RewardsManagerPubkey: rm,
Authorities: canonicalAuthorities,
# Conflicts:
#	pkg/core/server/rewards.go
@rickyrombo rickyrombo closed this May 12, 2026
rickyrombo added a commit that referenced this pull request May 12, 2026
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>
rickyrombo added a commit that referenced this pull request May 12, 2026
* Reward authority rotation primitive (PRs 222 + 225 + 228 bundled)

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>

* Tighten reward-pool gating: union replay, AUDIO-only fallback

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>

* CreateRewardPool: require ed25519 signature from RM keypair

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>

* Move rm_owner_signature to envelope, sign body bytes

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>

* Renumber reward-pools migration 00033 → 00034

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>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Address PR #254 review feedback

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>

* Address PR #254 review feedback (round 2)

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>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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