From 25132a20e9796c112479d327efc13a9a1fcb5520 Mon Sep 17 00:00:00 2001 From: Lev Akhnazarov Date: Fri, 17 Apr 2026 17:39:50 +0400 Subject: [PATCH 1/3] refactor(wallet-registry): update withdrawRewards documentation and add tests for allowlist integration - Enhanced documentation for the function to clarify the beneficiary retrieval process based on the current authorization source. - Removed historical migration rationale as it is no longer relevant. - Added new tests to verify reward distribution when the allowlist is active, ensuring correct beneficiary handling and reward payouts. --- solidity/ecdsa/contracts/WalletRegistry.sol | 30 ++++----- .../ecdsa/test/WalletRegistry.Rewards.test.ts | 64 +++++++++++++++++++ 2 files changed, 75 insertions(+), 19 deletions(-) diff --git a/solidity/ecdsa/contracts/WalletRegistry.sol b/solidity/ecdsa/contracts/WalletRegistry.sol index 757f457566..21a0056b21 100644 --- a/solidity/ecdsa/contracts/WalletRegistry.sol +++ b/solidity/ecdsa/contracts/WalletRegistry.sol @@ -450,32 +450,24 @@ 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(); diff --git a/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts b/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts index 972f675a15..92d52f9e8f 100644 --- a/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts +++ b/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts @@ -243,4 +243,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) + }) + }) }) From 0ff7d79bc2ffcdd997fc4a73366ebd8260a1c5b0 Mon Sep 17 00:00:00 2001 From: Lev Akhnazarov Date: Fri, 17 Apr 2026 17:48:21 +0400 Subject: [PATCH 2/3] refactor(wallet-registry): improve test structure and readability - Updated import statements to include 'ethers' alongside 'helpers' for clarity. - Refactored test code for better readability by adjusting line breaks and formatting. - Ensured consistent handling of the 'stakingProvider' variable for improved code maintainability. --- solidity/ecdsa/test/WalletRegistry.Rewards.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts b/solidity/ecdsa/test/WalletRegistry.Rewards.test.ts index 92d52f9e8f..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, @@ -282,17 +283,17 @@ describe("WalletRegistry - Rewards", () => { it("should pay rewards to _currentAuthorizationSource().rolesOf beneficiary (Allowlist)", async () => { const operator = membersLocal[0].signer.address - const stakingProvider = await walletRegistryLocal.operatorToStakingProvider( - operator - ) + 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) + const { beneficiary: expectedBeneficiary } = await allowlist.rolesOf( + stakingProvider + ) expect(expectedBeneficiary).to.equal(stakingProvider) await tTokenLocal From 0c6f839fe9cc5598c98ebd81dea2edec4c68fc4e Mon Sep 17 00:00:00 2001 From: Lev Akhnazarov Date: Fri, 17 Apr 2026 19:45:34 +0400 Subject: [PATCH 3/3] fix(wallet-registry): update beneficiary retrieval in withdrawRewards function - Changed the beneficiary retrieval logic in the withdrawRewards function to use the current authorization source, improving accuracy in reward distribution. - This change ensures that the correct beneficiary is identified based on the latest authorization context. --- solidity/ecdsa/contracts/WalletRegistry.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/solidity/ecdsa/contracts/WalletRegistry.sol b/solidity/ecdsa/contracts/WalletRegistry.sol index 21a0056b21..dcdbfafc30 100644 --- a/solidity/ecdsa/contracts/WalletRegistry.sol +++ b/solidity/ecdsa/contracts/WalletRegistry.sol @@ -471,7 +471,9 @@ contract WalletRegistry is 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);