You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix: prevent stuck withdrawals when Election cap exhausts a group (#235)
* fix: prevent and recover stuck withdrawals (Election cap exhaustion)
Rebased onto releases/4 for release.
Root cause: Account.scheduleWithdrawals validated per-group capacity via
getCeloForGroup, which counts scheduledVotes.toVote even when the Election
numVotesReceivable cap permanently blocks it. Withdrawals scheduled against
that stale toVote could never be revoked, pinning a user's CELO to a group
with no recovery path (forum.celo.org/t/.../13333).
Fix:
- New Account.getRealisableCeloForGroup = max(0, revokable - toRevoke)
+ min(balance, toVote), minus existing toWithdraw once (withdraw is
immediate-first, so a pending withdrawal costs one unit of combined
capacity, not one per bucket).
- scheduleWithdrawals books each entry through _addBeneficiaryWithdrawal,
which validates against realisable and threads a shared balance budget so
a multi-group call cannot double-count the same unlocked CELO.
- DefaultStrategy / SpecificGroupStrategy withdrawal distribution cap each
group by realisable (real withdrawals) and skip zero-capacity / already
chosen groups, so distribution never pins to a group that cannot pay and
never emits a group twice. Transfers (isTransfer) stay accounting-only.
- New permissionless Account.rescueScheduledWithdrawal re-routes an existing
stuck pin to groups that can fulfill it, gated by _requireGroupInDeficit so
it only fires when the group genuinely cannot pay the beneficiary now.
Versions bumped per the contract-compatibility check against releases/4:
Account 1.2.2.0 (minor: added view + rescue), DefaultStrategy 1.3.0.0
(major: generateWithdrawalVoteDistribution gained isTransfer),
SpecificGroupStrategy 1.1.1.1 and Manager 1.3.1.1 (patch).
Tests: account-realisable.test.ts (realisable + single-subtraction +
rescue), end-to-end-stuck-withdrawal.test.ts (real-contract prevention,
distribution, no-duplicate-group, rescue gating, shared balance budget),
plus scripts/verify-stuck-withdrawal-mainnet-fork.sh (anvil mainnet-fork
proof). Full suite green; compatibility check passes against releases/4.
* ci: run CI-Pipeline for release branches (releases/**), not just master
PR 235 targets releases/4; the workflow only triggered on PRs into master,
so retargeting left it without CI. Add releases/** to the push and
pull_request branch filters so release-targeted PRs get lint, test, and the
contract-compatibility check.
* fix: align contract versions with releases/4 compatibility baseline
The contract-compatibility CI job compares against a baseline one increment
below releases/4 source and requires getVersionNumber to land exactly on the
releases/4 source values (Account 1.2.1.0, DefaultStrategy 1.2.0.0,
SpecificGroupStrategy 1.1.1.0, Manager 1.3.1.0). An earlier local run read the
baseline one step higher (different toolchain) and over-bumped; revert to the
CI-mandated values.
* chore: add governance release script for stuck-withdrawal fix
Releases the 4 upgraded stCELO impls (Account, DefaultStrategy,
SpecificGroupStrategy, Manager) via a Celo Governance proposal. The proxies
are owned by the stCELO MultiSig, which exposes governanceProposeAndExecute
(onlyGovernance) - so a single CGP tx (Governance -> MultiSig
governanceProposeAndExecute([4 proxies],[0,0,0,0],[upgradeTo x4])) performs all
four upgrades, no owner timelock.
The script deploys the impls, builds the CGP calldata + JSON, and (default
fork-test mode) impersonates Celo Governance to execute it and assert: all 4
proxies repointed, pre-existing storage intact, new code live, versions at
release values. Storage layout verified unchanged vs releases/4 (solc layout
diff + CI compat + on-fork state-intact). --emit-only prints the CGP without
executing.
* chore: release script supports fork + mainnet, with explorer verification
- 'fork' (default): unchanged - fork mainnet, deploy ephemeral impls,
impersonate Celo Governance, execute governanceProposeAndExecute, assert
upgrades + storage intact + new code live.
- 'mainnet': real broadcast (needs DEPLOYER_PK, CELO_SCAN_API_KEY). Deploys
the 4 impls + AddressSortedLinkedList library, verifies each on Celoscan
(hardhat verify) AND Blockscout (forge verify-contract --verifier blockscout),
then builds + prints the CGP tx/JSON with the real addresses. Does not
execute the upgrade (Governance does, on vote pass).
DefaultStrategy verification passes the library link on both verifiers.
solc 0.8.11 / evm istanbul / optimizer disabled (matches hardhat.config).
* fix: correct DefaultStrategy library link + surface deploy errors
The library-link step reused norm() (strips leading zeros) to substitute the
deployed AddressSortedLinkedList address into DefaultStrategy's bytecode
placeholder. If the library address has a leading zero this produces a
wrong-length (malformed) link and the deploy fails - which is what happened on
the mainnet attempt. Preserve the full 40-hex address (0x stripped, lowercased,
leading zeros kept) and assert no unlinked placeholder remains.
Also: deploy() no longer swallows cast stderr - it prints the real error and
aborts, so a failed deploy (bad link, gas, or insufficient funds) is diagnosable
instead of an empty address.
* fix: explicit incrementing nonce for sequential deploys
forno's 'latest' nonce can lag the sequencer, so cast auto-nonce submitted a
stale value (nonce too low: next 28, tx 27). Anchor to the PENDING nonce once
and pass an explicit, caller-incremented --nonce for each of the 5 deploys so
they stay in lock-step with the sequencer. Nonce is incremented in the caller,
not inside the deploy function (it runs in a command-substitution subshell).
* fix: verify Celoscan via forge (avoid hardhat-etherscan solc-bin DNS dep)
hardhat verify failed with 'getaddrinfo ENOTFOUND solc-bin.ethereum.org' - the
@nomiclabs/hardhat-etherscan plugin fetches the solc version list from that host
before verifying. Switch the Celoscan step to forge verify-contract --chain celo
(same as Blockscout, which passed), which uses the local solc and has no
solc-bin dependency. Both explorers now verified through forge.
---------
Co-authored-by: Pavel Hornak <pavel.hornak@clabs.co>
0 commit comments