diff --git a/solidity/ecdsa/contracts/WalletRegistry.sol b/solidity/ecdsa/contracts/WalletRegistry.sol index 757f457566..dcdbfafc30 100644 --- a/solidity/ecdsa/contracts/WalletRegistry.sol +++ b/solidity/ecdsa/contracts/WalletRegistry.sol @@ -450,36 +450,30 @@ contract WalletRegistry is } /// @notice Withdraws application rewards for the given staking provider. - /// Rewards are withdrawn to the staking provider's beneficiary - /// address set in the staking contract. Reverts if staking provider - /// has not registered the operator address. + /// Rewards are sent to the beneficiary returned by + /// `_currentAuthorizationSource().rolesOf(stakingProvider)` — the + /// same authorization source used elsewhere after TIP-092 (Allowlist + /// when `allowlist != address(0)`, otherwise TokenStaking). Reverts + /// if the staking provider has not registered an operator address. /// @dev Emits `RewardsWithdrawn` event. /// - /// NOT MIGRATED: Beneficiary lookup remains on TokenStaking because - /// migrating dead code costs 50-100 bytes with zero benefit. + /// Allowlist vs TokenStaking: `Allowlist.rolesOf` follows TIP-092 semantics + /// (beneficiary is the staking provider address), while `TokenStaking.rolesOf` + /// can return a distinct delegated beneficiary. After `initializeV2(allowlist)`, + /// reward payout follows Allowlist, not legacy TokenStaking role resolution. /// /// Historical Context (TIP-092/100 - February 15, 2025): /// - Sortition pool DKG participation rewards HALTED Feb 15, 2025 /// - TokenStaking notification rewards HALTED for ECDSA/RandomBeacon /// - Only TACo application rewards continue (6-month transition) - /// - This function now returns 0 for all ECDSA operators (no rewards) - /// - /// Migration Decision Rationale: - /// - Bytecode cost: 50-100 bytes to migrate beneficiary lookup to Allowlist - /// - Benefit: Zero (function returns 0 - no rewards to withdraw) - /// - Preserved for historical compatibility and potential future reactivation - /// - /// Technical Note: If rewards are reactivated, Allowlist migration would - /// be required as Allowlist.rolesOf() always returns stakingProvider as - /// beneficiary (no delegation support), while TokenStaking.rolesOf() - /// returns configured beneficiary (supports owner != beneficiary delegation). - /// - /// Stakeholder Decision: Pragmatic choice to avoid bytecode cost for - /// dead code, predating TIP-092/100 implementation. + /// - For ECDSA, expect zero withdrawable amount unless application rewards + /// are reactivated. function withdrawRewards(address stakingProvider) external { address operator = stakingProviderToOperator(stakingProvider); if (operator == address(0)) revert UnknownOperator(); - (, address beneficiary, ) = staking.rolesOf(stakingProvider); + (, address beneficiary, ) = _currentAuthorizationSource().rolesOf( + stakingProvider + ); uint96 amount = sortitionPool.withdrawRewards(operator, beneficiary); // slither-disable-next-line reentrancy-events emit RewardsWithdrawn(stakingProvider, amount); diff --git a/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts b/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts index e34d2d60b6..4acb4ad45f 100644 --- a/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts +++ b/solidity/ecdsa/test/WalletRegistry.Authorization.test.ts @@ -3714,7 +3714,7 @@ describe("WalletRegistry - Authorization", () => { * Test Coverage: * - Pre-upgrade mode: Authorization routes to TokenStaking (allowlist = address(0)) * - Post-upgrade mode: Authorization routes to Allowlist (allowlist != address(0)) - * - NOT MIGRATED touchpoints: Slashing and beneficiary queries stay on TokenStaking + * - NOT MIGRATED touchpoints: Slashing stays on TokenStaking * - Upgrade flow: Transition from TokenStaking to Allowlist via initializeV2() * - Edge cases: Zero address validation, re-initialization prevention * @@ -3975,74 +3975,6 @@ describe("WalletRegistry - Migration Scenario Tests (TIP-092)", () => { }) }) - /** - * NOT MIGRATED Touchpoint Tests - * - * Context: Some functions do NOT use _currentAuthorizationSource(). - * Expected: These functions always use staking contract, even after initializeV2(). - * - * Rationale: - * - withdrawRewards: Beneficiary roles remain in TokenStaking (WalletRegistry.sol:440-452) - * - challengeDkgResult: Stake custody and slashing remain in TokenStaking (WalletRegistry.sol:950-966) - */ - describe("NOT MIGRATED Touchpoints", () => { - let allowlist: FakeContract - - before(async () => { - await createSnapshot() - - // Setup: Use real TokenStaking for beneficiary roles (NOT migrated to Allowlist) - await setupRealStaking( - t, - staking, - walletRegistry, - deployer, - stakingProvider, - beneficiary, - minimumAuthorization - ) - - // Setup: Create allowlist fake and upgrade (but beneficiary still in TokenStaking) - allowlist = await smock.fake("IStaking") - allowlist.authorizedStake.returns(minimumAuthorization) - await walletRegistry.initializeV2(allowlist.address) - - // Setup: Trigger authorization callback from allowlist (post-upgrade) - await triggerAuthorizationCallback( - walletRegistry, - allowlist.address, - stakingProvider.address, - ethers.BigNumber.from(0), - minimumAuthorization - ) - - // Setup: Register operator with allowlist authorization - await walletRegistry - .connect(stakingProvider) - .registerOperator(operator.address) - }) - - after(async () => { - await restoreSnapshot() - }) - - /** - * Test: withdrawRewards always uses staking.rolesOf() for beneficiary lookup - * NOT using _currentAuthorizationSource() - * Direct call: staking.rolesOf() at line 456 - */ - it("should query TokenStaking for beneficiary in withdrawRewards (post-upgrade)", async () => { - // This test verifies that even after initializeV2, beneficiary lookup - // goes to TokenStaking, not Allowlist - expect(await walletRegistry.allowlist()).to.equal(allowlist.address) - - // Note: withdrawRewards requires actual rewards to test fully - // This test validates the pattern - beneficiary lookup stays on TokenStaking - const roles = await staking.rolesOf(stakingProvider.address) - expect(roles.beneficiary).to.equal(beneficiary.address) - }) - }) - /** * Upgrade Flow Tests * diff --git a/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts b/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts index 972f675a15..6a0040b92b 100644 --- a/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts +++ b/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts @@ -1,4 +1,4 @@ -import { helpers } from "hardhat" +import { ethers, helpers } from "hardhat" import { expect } from "chai" import { walletRegistryFixture } from "./fixtures" @@ -10,6 +10,7 @@ import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers" import type { FakeContract } from "@defi-wonderland/smock" import type { Operator, OperatorID } from "./utils/operators" import type { + Allowlist, SortitionPool, WalletRegistry, WalletRegistryGovernance, @@ -243,4 +244,68 @@ describe("WalletRegistry - Rewards", () => { }) }) }) + + describe("withdrawRewards when allowlist != address(0)", () => { + const walletPublicKeyLocal: string = ecdsaData.group1.publicKey + + let tTokenLocal: T + let walletRegistryLocal: WalletRegistry + let sortitionPoolLocal: SortitionPool + let randomBeaconLocal: FakeContract + let walletOwnerLocal: FakeContract + let deployerLocal: SignerWithAddress + let membersLocal: Operator[] + + before(async () => { + await createSnapshot() + ;({ + tToken: tTokenLocal, + walletRegistry: walletRegistryLocal, + sortitionPool: sortitionPoolLocal, + randomBeacon: randomBeaconLocal, + walletOwner: walletOwnerLocal, + deployer: deployerLocal, + } = await walletRegistryFixture({ useAllowlist: true })) + expect(await walletRegistryLocal.allowlist()).to.not.equal( + ethers.constants.AddressZero + ) + ;({ members: membersLocal } = await createNewWallet( + walletRegistryLocal, + walletOwnerLocal.wallet, + randomBeaconLocal, + walletPublicKeyLocal + )) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should pay rewards to _currentAuthorizationSource().rolesOf beneficiary (Allowlist)", async () => { + const operator = membersLocal[0].signer.address + const stakingProvider = + await walletRegistryLocal.operatorToStakingProvider(operator) + const allowlistAddr = await walletRegistryLocal.allowlist() + const allowlist = (await ethers.getContractAt( + "Allowlist", + allowlistAddr + )) as Allowlist + + const { beneficiary: expectedBeneficiary } = await allowlist.rolesOf( + stakingProvider + ) + expect(expectedBeneficiary).to.equal(stakingProvider) + + await tTokenLocal + .connect(deployerLocal) + .mint(deployerLocal.address, rewardAmount) + await tTokenLocal + .connect(deployerLocal) + .approveAndCall(sortitionPoolLocal.address, rewardAmount, []) + + expect(await tTokenLocal.balanceOf(expectedBeneficiary)).to.equal(0) + await walletRegistryLocal.withdrawRewards(stakingProvider) + expect(await tTokenLocal.balanceOf(expectedBeneficiary)).to.be.gt(0) + }) + }) })