Skip to content

Cutover validator endpoints to per-RM pool gate (PR 3) - #228

Closed
rickyrombo wants to merge 7 commits into
mjp-reward-pools-txfrom
mjp-reward-pools-endpoints
Closed

Cutover validator endpoints to per-RM pool gate (PR 3)#228
rickyrombo wants to merge 7 commits into
mjp-reward-pools-txfrom
mjp-reward-pools-endpoints

Conversation

@rickyrombo

@rickyrombo rickyrombo commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Wires up the rotation flow on top of #222 (PR 1 — schema) and #225 (PR 2 — pool primitive + cometbft txs). Once this lands, an OAP authority change for a launchpad coin's pool can be followed by validator-signed Solana CreateSenderPublic / DeleteSenderPublic instructions, bringing the on-chain reward manager into agreement with the OAP-side authority list.

This is PR 3 of 3 in the reward-authority rotation initiative.

Dispatch: senderGateForRM

Single helper resolves which gating regime applies to a given Solana reward manager pubkey:

senderGateForRM(rm) →
  if exists pool with rewards_manager_pubkey == rm:
    return (pool, true, nil)        // per-RM pool gate
  else:
    return (nil, false, nil)        // fall through to validator/AAO trust set

Transient DB errors are propagated rather than silently downgrading — a temporary blip must not weaken the gate from per-RM to network-wide.

Cutover

GetRewardSenderAttestation

  • Pool-managed RM: sign iff addr ∈ pool.authorities.
  • Non-pool RM (AUDIO): existing validator/AAO check.

GetDeleteRewardSenderAttestation

  • Pool-managed RM: sign iff addr ∉ pool.authorities. This is the rotation-out signal — once OAP removes a key from its pool, validators can sign a Solana DeleteSenderPublic attestation, deregistering the rotated-out key from on-chain attestation duties.
  • Non-pool RM (AUDIO): existing "must NOT be a validator/AAO" check.

GetRewardAttestation restored

Removes the 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 PR 1's LEFT JOIN. Rotating an authority out via SetRewardPoolAuthorities immediately revokes their ability to authenticate claim attestations — no separate code path needed.

PR 1 backfilled all existing rewards to RM-bound pools using the launchpad mapping in launchpad_authority_rm, so the leaked-key attestation vector closes the moment an authority is rotated via SetRewardPoolAuthorities.

AUDIO RM denylist

config.AudioRewardsManagerPubkey() returns the AUDIO RM pubkey for the current runtime environment:

  • Dev: DJPzVothq58SmkpRb1ATn5ddN2Rpv1j2TcGvM3XsHf1c
  • Stage: intentionally unset
  • Prod: 71hWFVYokLaN1PNYzTAWi13EfJ7Xt9VbSWUKsXUT8mxE

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; senderGateForRM would resolve AUDIO sender requests to the attacker-controlled pool, validators would sign, and the attacker could become an AUDIO sender on Solana. The denylist closes that vector by structurally preventing the AUDIO RM from ever having a pool.

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.

Test plan

  • go build ./... clean.
  • go vet ./pkg/core/... — no new warnings.
  • Unit tests for validateRewardsManagerPubkey (pkg/core/server/reward_pools_test.go):
    • shape rejections (empty, whitespace, non-base58, wrong length)
    • AUDIO denylist (configured AUDIO RM rejected; other RMs pass)
  • Integration tests (pkg/integration_tests/13_reward_pools_test.go) — full rotation flow against validator endpoints:
    • CreateSender attestation signed for current pool authority
    • CreateSender attestation refused for rotated-out address
    • DeleteSender attestation signed for rotated-out address
    • DeleteSender attestation refused for current pool authority
  • 12_rewards_test.go skip markers removed now that GetRewardAttestation is restored.
  • Devnet smoke: full rotation cycle end-to-end.

🤖 Generated with Claude Code

@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from b9d1b34 to 3669e95 Compare May 7, 2026 00:26
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from 4569adb to b9135cc Compare May 7, 2026 00:32
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from 3669e95 to da41064 Compare May 7, 2026 00:50
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from b9135cc to 62f6dd7 Compare May 7, 2026 00:51
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from da41064 to ea6e545 Compare May 7, 2026 00:54
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from 62f6dd7 to 310e044 Compare May 7, 2026 00:55
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from ea6e545 to 816f923 Compare May 7, 2026 01:11
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from 310e044 to 3cf1a9a Compare May 7, 2026 01:12
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>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-tx branch from 816f923 to 63d60c9 Compare May 7, 2026 02:52
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from 3cf1a9a to 8c4c696 Compare May 7, 2026 02:55
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>
@rickyrombo
rickyrombo requested a review from Copilot May 7, 2026 02:57

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 completes the reward-authority rotation initiative by cutting validator attestation endpoints over to per–reward-manager (RM) reward-pool gating (with a safe fallback to the legacy validator/AAO trust set for unmanaged RMs like AUDIO), restoring programmatic reward attestations, and adding SDK + test coverage for the new rotation flow.

Changes:

  • Add per-RM sender attestation dispatch (senderGateForRM) so Create/Delete sender attestations gate on core_reward_pools.authorities when a pool exists, else fall back to validator/AAO eligibility.
  • Restore GetRewardAttestation now that claim authorities are pool-backed (post-PR1 LEFT JOIN).
  • Add AUDIO RM denylist for CreateRewardPool, plus SDK helpers and integration/unit tests for rotation flows.

Reviewed changes

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

Show a summary per file
File Description
pkg/core/server/connect.go Restores GetRewardAttestation and adds per-RM pool gating for sender attestations via senderGateForRM.
pkg/core/server/reward_pools.go Adds AUDIO RM denylist enforcement in validateRewardsManagerPubkey.
pkg/core/config/rewards.go Introduces env-specific AUDIO RM pubkey configuration used by the denylist.
pkg/sdk/rewards/rewards.go Adds SDK wrappers for GetRewardSenderAttestation / GetDeleteRewardSenderAttestation.
pkg/core/server/reward_pools_test.go Adds unit tests for RM pubkey validation and AUDIO denylist behavior.
pkg/integration_tests/13_reward_pools_test.go Adds end-to-end integration coverage for Create/Delete sender attestation rotation behavior.
pkg/integration_tests/12_rewards_test.go Removes skip markers now that GetRewardAttestation is restored.

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

Comment thread pkg/core/server/connect.go Outdated
Comment thread pkg/core/server/connect.go Outdated
Comment thread pkg/core/server/reward_pools.go Outdated
Comment thread pkg/core/config/rewards.go
Comment thread pkg/core/server/reward_pools_test.go Outdated
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 63d60c9 to 9d1f73d Compare May 7, 2026 05:48
rickyrombo and others added 2 commits May 6, 2026 22:48
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>
Both decode to 32 bytes via base58. Stage intentionally left empty per
direction — staging traffic that targets the prod-shape AUDIO RM falls
through to the validator/AAO gate (the existing behavior), which is the
correct outcome since staging-side rotation will be exercised against
test launchpad RMs that have first-class pools.

  Dev:  DJPzVothq58SmkpRb1ATn5ddN2Rpv1j2TcGvM3XsHf1c
  Prod: 71hWFVYokLaN1PNYzTAWi13EfJ7Xt9VbSWUKsXUT8mxE

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from 8c4c696 to 2188197 Compare May 7, 2026 05:50
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-tx branch from 9d1f73d to 2f5a5f0 Compare May 8, 2026 01:04
@rickyrombo
rickyrombo force-pushed the mjp-reward-pools-endpoints branch from 2188197 to 2645ab1 Compare May 8, 2026 01:12
# Conflicts:
#	pkg/core/server/connect.go
#	pkg/core/server/reward_pools.go
#	pkg/integration_tests/13_reward_pools_test.go
#	pkg/sdk/rewards/rewards.go
@rickyrombo
rickyrombo requested a review from Copilot May 8, 2026 01:31

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

Comments suppressed due to low confidence (1)

pkg/sdk/rewards/rewards.go:197

  • SDK signing for GetRewardAttestation does not include req.RewardAddress in the pkgrewards.RewardClaim it signs. If the server starts binding programmatic-reward claims to RewardAddress (recommended to prevent cross-reward replay), this client will produce unverifiable signatures; also, leaving RewardAddress out keeps the replay surface open when reward_id/specifier collide across rewards.
func (r *Rewards) GetRewardAttestation(ctx context.Context, req *v1.GetRewardAttestationRequest) (*v1.GetRewardAttestationResponse, error) {
	// Create a RewardClaim to compile the data in the correct format
	claim := pkgrewards.RewardClaim{
		RecipientEthAddress: req.EthRecipientAddress,
		Amount:              req.Amount,
		RewardID:            req.RewardId,
		Specifier:           req.Specifier,
		ClaimAuthority:      req.ClaimAuthority, // Use claim authority as oracle
		Decimals:            req.AmountDecimals,
	}

Comment thread pkg/core/server/connect.go
Comment thread pkg/integration_tests/12_rewards_test.go
…h tests

Two review-driven fixes:

1. GetRewardAttestation comment: add a comment next to the RewardClaim
   construction explaining why RewardAddress is intentionally NOT set
   even though Compile supports a 3-piece RewardAddress:RewardID:Specifier
   form. 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 on this side would break on-chain signature verification,
   not just change the validator-side signing contract. Cross-reward
   replay protection therefore relies on Specifier being disbursement-
   unique (per recipient + per event) — the existing contract.

2. Reward attestation integration tests: tests 3 and 4 in "Test Reward
   Attestations with Claim Authorities" passed but masked a regression.
   Validate runs before Authenticate and rejects Amount mismatch first,
   so the prior amounts (1000 and 500 vs reward amounts 5000 and 3000)
   meant the tests were failing at amount validation, never reaching the
   auth gate. Aligned amounts with the rewards and added explicit
   connect.CodePermissionDenied assertions so a regression that drops
   the auth check would actually fail the test.

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

Comments suppressed due to low confidence (1)

pkg/core/server/connect.go:1866

  • The error returned from rewards.GetCreateSenderAttestation is discarded and the RPC responds with a generic "could not create attestation". Keeping the underlying error (or at least its message) would make malformed addresses / invalid base58 pubkeys much easier to diagnose for clients and in logs.
	owner, attestation, err := rewards.GetCreateSenderAttestation(c.core.config.EthereumKey, &rewards.CreateSenderAttestationParams{
		NewSenderAddress:            address,
		RewardsManagerAccountPubKey: rewardsManagerPubkey,
	})

Comment on lines +1832 to 1835
address := strings.TrimSpace(req.Msg.Address)
if address == "" {
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("address is required"))
}
Comment on lines +1837 to 1840
rewardsManagerPubkey := strings.TrimSpace(req.Msg.RewardsManagerPubkey)
if rewardsManagerPubkey == "" {
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("reward manager pubkey is required"))
}
Comment on lines +1887 to 1890
address := strings.TrimSpace(req.Msg.Address)
if address == "" {
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("address is required"))
}
Comment on lines +1892 to 1895
rewardsManagerPubkey := strings.TrimSpace(req.Msg.RewardsManagerPubkey)
if rewardsManagerPubkey == "" {
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("reward manager pubkey is required"))
}
return nil, connect.NewError(connect.CodeNotFound, errors.New("programmatic reward not found"))
}
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to load reward: %w", err))
}
}
}

owner, attestation, err := rewards.GetDeleteSenderAttestation(c.core.config.EthereumKey, &rewards.DeleteSenderAttestationParams{
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.

2 participants