fix(payments): split refund out of TanglePaymentsFacet to clear EIP-170#149
Merged
Conversation
Stranded from #147 — the Cargo.toml bumps landed but the corresponding Cargo.lock regen was never committed. No transitive dependency changes, just the two workspace member versions.
`TanglePaymentsFacet`'s runtime bytecode reached 24874 bytes (298 over the 24576-byte EIP-170 limit) after PR #147 added `withdrawRemainingEscrowTo`. Local anvil runs with `--disable-code-size-limit` so the regression slipped through, but Base Sepolia (and every other production chain) enforces the limit strictly — the facet would not deploy. Extract `withdrawRemainingEscrow{,To}` and the private `_withdrawRemainingEscrow` helper into a new `PaymentsRefund` mixin and route those two selectors via `TanglePaymentsRewardsFacet`, which already owns the rewards/admin/views surface and had 8.8 KB of headroom. The customer-facing funding + billing facet shrinks by 770 bytes and lands 472 bytes under the limit. Sizes (default profile, optimizer on): TanglePaymentsFacet: 24874 → 24104 (-770, headroom 472) TanglePaymentsRewardsFacet: 15751 → 16280 (+529, headroom 8296) TanglePaymentsDistributionFacet: 19651 → 19651 (unchanged) The diamond router wires selectors from each facet's `selectors()` view, so `Tangle.sol`'s public surface is unchanged: `withdrawRemainingEscrow{,To}` still dispatch through the same calldata path, just to a different facet. The `ITangleServices.sol` interface is unaffected. Tests: - test/tangle/Payments.t.sol::WithdrawRemainingEscrowTo (3/3 pass) covers the new routing through TanglePaymentsRewardsFacet, the ZeroAddress revert, and the NotServiceOwner gate. - test/EndToEndSubscriptionTest.t.sol (9/9 pass) covers the full lifecycle including post-termination withdrawal. - test/tangle/SubscriptionEscrowInvariant.t.sol (3/3 pass) exercises 3,837 `refundRemaining` handler invocations against the new routing.
Contributor
Author
🔍 Reviewing
|
| Pass | Status | ETA |
|---|---|---|
| Kimi Code K2.6 | Running (4 min) | ~5-15 min |
| opencode GLM 5.1 | Running (4 min) | ~5-15 min |
Agent review running. Reads the actual code. This comment updates in place.
tangletools · #149 · model: kimi-for-coding · started 2026-05-17T22:38:46Z
3 tasks
tangletools
pushed a commit
that referenced
this pull request
May 20, 2026
Asserts every deployable Tangle and staking facet has runtime bytecode under the 24576-byte EIP-170 limit. Catches the exact regression PR #149 had to clean up: `TanglePaymentsFacet` shipped at 24874 bytes because the local stress harness runs anvil with `--disable-code-size-limit`, so the test suite never deployed it on a chain that enforces the limit. The check measures `address(new Facet()).code.length` after constructor under the default profile (optimizer on, via-IR) — i.e. the actual bytecode a real broadcast would deploy. Two test functions (one per diamond family) so a single regression surfaces only the failing family. Current headroom for the tighter facets: TanglePaymentsFacet: 24104 (472 under) TangleServicesFacet: ~21k (~3.5k under, monitor) TanglePaymentsDistributionFacet: 19651 (4925 under) A future PR that grows TanglePaymentsFacet by >472 bytes — or any similar regression — will now fail CI on PR with a precise overage number rather than reverting at deploy time on the first production broadcast.
drewstone
added a commit
that referenced
this pull request
May 20, 2026
…eanup (#150) * chore(cargo): sync lockfile to tnt-core-bindings/fixtures v0.17.1 Stranded from #147 — the Cargo.toml bumps landed but the corresponding Cargo.lock regen was never committed. No transitive dependency changes, just the two workspace member versions. * fix(payments): split refund out of TanglePaymentsFacet to clear EIP-170 `TanglePaymentsFacet`'s runtime bytecode reached 24874 bytes (298 over the 24576-byte EIP-170 limit) after PR #147 added `withdrawRemainingEscrowTo`. Local anvil runs with `--disable-code-size-limit` so the regression slipped through, but Base Sepolia (and every other production chain) enforces the limit strictly — the facet would not deploy. Extract `withdrawRemainingEscrow{,To}` and the private `_withdrawRemainingEscrow` helper into a new `PaymentsRefund` mixin and route those two selectors via `TanglePaymentsRewardsFacet`, which already owns the rewards/admin/views surface and had 8.8 KB of headroom. The customer-facing funding + billing facet shrinks by 770 bytes and lands 472 bytes under the limit. Sizes (default profile, optimizer on): TanglePaymentsFacet: 24874 → 24104 (-770, headroom 472) TanglePaymentsRewardsFacet: 15751 → 16280 (+529, headroom 8296) TanglePaymentsDistributionFacet: 19651 → 19651 (unchanged) The diamond router wires selectors from each facet's `selectors()` view, so `Tangle.sol`'s public surface is unchanged: `withdrawRemainingEscrow{,To}` still dispatch through the same calldata path, just to a different facet. The `ITangleServices.sol` interface is unaffected. Tests: - test/tangle/Payments.t.sol::WithdrawRemainingEscrowTo (3/3 pass) covers the new routing through TanglePaymentsRewardsFacet, the ZeroAddress revert, and the NotServiceOwner gate. - test/EndToEndSubscriptionTest.t.sol (9/9 pass) covers the full lifecycle including post-termination withdrawal. - test/tangle/SubscriptionEscrowInvariant.t.sol (3/3 pass) exercises 3,837 `refundRemaining` handler invocations against the new routing. * chore(deploy): prune base-sepolia config for actual broadcast `deploy/config/base-sepolia.json` carried 8 stake-asset entries with zero token addresses (stETH, wstETH, EIGEN, USDT, DAI, WBTC, tBTC, lBTC) — not available on Base Sepolia. `FullDeploy.s.sol:818` silently skips them, but they clutter the post-deploy manifest and obscure what's actually live. Keep TNT (sentinel), WETH (`0x4200…0006`), USDC (`0x036C…3cF7e`). Other changes: - `roles.revokeBootstrap`: `false` → `true`. Deployer should not retain bootstrap privileges on a "real" testnet validation deployment. - Strip the `_todo_*` documentation keys — they were planning notes, not config the deploy script reads. Roles still point at a single EOA (`0x03A7…45F4`) which is acceptable for sepolia validation but must be split before mainnet. Migration deploy is left enabled so we exercise the full claim infrastructure on sepolia. * fix(deploy): 2-phase bootstrap so `admin != deployer` deploys work A dry-run of `FullDeploy.s.sol` against real Base Sepolia state surfaced a latent blocker: the script reverts at the first `registerFacet()` call when `admin != deployer`. `__Base_init` and `MultiAssetDelegation.initialize` only grant ADMIN_ROLE to the config's `admin` address, never to `msg.sender` (the deployer). After init, the deployer has zero roles and cannot register facets, grant cross-contract roles, or call any admin-gated setup function from its broadcast context. Local-env (anvil) hides this because `local.anvil.json` sets `admin: 0x000…000`, which `FullDeploy` substitutes to the deployer at runtime — so `admin == deployer` and the script accidentally works. Production chains (mainnet, Base mainnet, Arbitrum, Optimism, Tangle) are even more affected: `_requireProductionRoles` *enforces* `admin != deployer`, so the previous code could never deploy to any production chain. Switch the deploy flow to a 2-phase bootstrap pattern: 1. **Bootstrap phase**: every proxy / contract is initialized with the *deployer* as the initial DEFAULT_ADMIN_ROLE / ADMIN_ROLE holder. The deployer can then register facets, grant `MANAGER_ROLE` on the MBSM registry, enable assets, wire the rewards manager, etc., all from its own broadcast context. 2. **Handoff phase**: existing `_applyRoleHandoff` already (a) grants permanent roles to `timelock` + `multisig` and (b) revokes the bootstrap admin's roles when `revokeBootstrap == true`. We just thread `deployer` as the `bootstrapAdmin` parameter so the right address gets revoked. `_assertGovernanceConfiguration` is updated in lockstep. No protocol contract changes — this is purely a script-side fix. The `admin` parameter from config now flows only into `_resolveCredits` (Credits is non-upgradeable Ownable, used as the final owner directly) and as the fallback handoff target when timelock/multisig are unset. Verified end-to-end against real Base Sepolia state via: PRIVATE_KEY=… FULL_DEPLOY_CONFIG=deploy/config/base-sepolia.json \ forge script script/FullDeploy.s.sol:FullDeploy \ --rpc-url https://sepolia.base.org The regenerated `deployments/base-sepolia/{latest,migration}.json` are included as evidence — every proxy + facet deployed cleanly, role handoff to the config admin succeeded, smoke tests passed, and the deployer ended with zero roles. With the EIP-170 facet split now on main (PR #149), the script produces a complete, byte-size-clean deployment that a real broadcast can sign. Also included on this branch (commit c8159f1): - Prune `deploy/config/base-sepolia.json`: drop 8 zero-token stake assets (stETH, wstETH, EIGEN, USDT, DAI, WBTC, tBTC, lBTC), strip stale `_todo_*` doc keys, flip `revokeBootstrap` to `true`. * test(facets): EIP-170 gate so anvil can't hide oversized facets Asserts every deployable Tangle and staking facet has runtime bytecode under the 24576-byte EIP-170 limit. Catches the exact regression PR #149 had to clean up: `TanglePaymentsFacet` shipped at 24874 bytes because the local stress harness runs anvil with `--disable-code-size-limit`, so the test suite never deployed it on a chain that enforces the limit. The check measures `address(new Facet()).code.length` after constructor under the default profile (optimizer on, via-IR) — i.e. the actual bytecode a real broadcast would deploy. Two test functions (one per diamond family) so a single regression surfaces only the failing family. Current headroom for the tighter facets: TanglePaymentsFacet: 24104 (472 under) TangleServicesFacet: ~21k (~3.5k under, monitor) TanglePaymentsDistributionFacet: 19651 (4925 under) A future PR that grows TanglePaymentsFacet by >472 bytes — or any similar regression — will now fail CI on PR with a precise overage number rather than reverting at deploy time on the first production broadcast. --------- Co-authored-by: Drew Stone <drewstone329@gmail.com>
2 tasks
drewstone
added a commit
that referenced
this pull request
May 20, 2026
The Foundry CI job has been red on every PR since the codebase grew past the ubuntu-latest runner's 16 GB memory budget. `forge build --sizes` gets OOM-killed (exit 143) during the 352-file via-IR compile around the 20-minute mark — same failure on every commit including the now-merged #149 and #150. Until we have access to a larger runner (or split the compile graph), this job is pure noise: it blocks no merge (PRs are merged over red CI as a matter of course) and surfaces no signal a developer can act on. EIP-170 facet sizes are now enforced via `test/FacetSize.t.sol` (added in #150), which is much cheaper to run and produces a precise overage number on regression. Local `forge test` remains the source of truth for correctness. Restore once we have a runner with enough RAM (or a leaner compile), or replace with a sharded matrix that fits each shard inside 16 GB. Co-authored-by: Drew Stone <drewstone329@gmail.com>
drewstone
added a commit
that referenced
this pull request
May 20, 2026
* chore(ci): remove broken Foundry CI The Foundry CI job has been red on every PR since the codebase grew past the ubuntu-latest runner's 16 GB memory budget. `forge build --sizes` gets OOM-killed (exit 143) during the 352-file via-IR compile around the 20-minute mark — same failure on every commit including the now-merged #149 and #150. Until we have access to a larger runner (or split the compile graph), this job is pure noise: it blocks no merge (PRs are merged over red CI as a matter of course) and surfaces no signal a developer can act on. EIP-170 facet sizes are now enforced via `test/FacetSize.t.sol` (added in #150), which is much cheaper to run and produces a precise overage number on regression. Local `forge test` remains the source of truth for correctness. Restore once we have a runner with enough RAM (or a leaner compile), or replace with a sharded matrix that fits each shard inside 16 GB. * chore(deploy): first Base Sepolia deployment (chainId 84532) Live deployment from the bootstrap-fix path landed in #150. Broadcast from `0x2420FFf17c4213A4075cf5f7B6dc33429Aaf22Bb` (the shared testnet deployer in `~/company/devops/secrets/shared-testnet-deployer.env`), total cost 0.0009 ETH on Base Sepolia. Key addresses: Tangle 0xC9b0716a187072be0f38A5D972392C6479b9Cfe3 MultiAssetDelegation 0xfEB417fc6d343e0fc88EC9fDb8294BF84d69F0Ca TNT Token 0x541d93d0650F8E4583B7Db6545cCeee649975831 RewardVaults 0x8AFfb8C215679210329ef9129d8427fA4E7bd087 InflationPool 0x2C44736AaF0EeC7C9b852eE71F0DC05b1606803b ServiceFeeDistributor 0xdA134D167a99fCb9bD5bb977F1F886D9401b1098 TangleMetrics 0xBBc1386b628716241b37e83F99C50A43878c42C2 TangleMigration 0x0f24429FF66B3fF61390888ef7C69F07533D4b01 SP1ZKVerifier 0x609C9Fb4B687D3f172719B13C0012530C003449E Credits 0xEeba50602c52096091cCe56cDB12cA1DF049542A OperatorStatusRegistry 0x81443688Fce1e4eDb822c1D5794C3DAc608e9a23 Role handoff verified on-chain via `cast call hasRole`: - deployer (0x2420…22Bb) → DEFAULT_ADMIN_ROLE = false (revoked) - admin (0x03A7…45F4) → DEFAULT_ADMIN_ROLE = true (granted) 109.25M TNT minted to admin. TNT / WETH / USDC enabled as staking assets. Migration deployed with the configured Merkle root + SP1 verifier (real, not mock). Single-EOA governance for sepolia (admin = treasury = timelock = multisig = 0x03A7…45F4) is acceptable for testnet validation but must be split before any mainnet broadcast. * chore(indexer): point base-sepolia config at first deployment Update indexer/config.yaml with the contract addresses from the live Base Sepolia broadcast that landed earlier in this branch. Populates MasterBlueprintServiceManager and Credits where they were previously empty (those contracts didn't exist in the prior partial deployment). Tangle 0x62281eac… → 0xC9b0716a… MultiAssetDelegation 0x96e682cc… → 0xfEB417fc… OperatorStatusRegistry 0x17746107… → 0x81443688… RewardVaults 0x37979744… → 0x8AFfb8C2… InflationPool 0x8152f13c… → 0x2C44736A… MBSM (none) → 0xf259444a… Credits (none) → 0xEeba5060… ValidatorPodManager / LiquidDelegationFactory / LiquidDelegationVault remain unset — those are separate beacon / liquid-staking deploys not part of the FullDeploy script. Indexer can now `npm run dev` against base-sepolia and start ingesting events at block 0 of the new deployment. --------- Co-authored-by: Drew Stone <drewstone329@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TanglePaymentsFacetruntime bytecode reached 24874 bytes (298 over the 24576-byte EIP-170 limit) after PR #147 addedwithdrawRemainingEscrowTo. Local anvil hides this because it runs with--disable-code-size-limit, but Base Sepolia and every other production chain enforces the limit strictly — the facet would not deploy.Extract the post-termination refund path into a new
PaymentsRefundmixin and route those two selectors viaTanglePaymentsRewardsFacet, which already owns the rewards/admin/views surface and had 8.8 KB of headroom.Sizes (default profile, optimizer on)
Net facet bytecode: −241 B.
Public surface
Unchanged. The diamond router wires selectors via each facet's
selectors()view, sowithdrawRemainingEscrow{,To}still dispatch through the same calldata path onTangle.sol, just to a different facet.ITangleServices.solinterface is not modified.Test plan
Payments.t.sol::WithdrawRemainingEscrowTo*— 3/3 pass (covers new routing,ZeroAddressrevert,NotServiceOwnergate)EndToEndSubscriptionTest.t.sol— 9/9 pass (full subscription lifecycle including post-termination refund)SubscriptionEscrowInvariant.t.sol— 3/3 pass (3,837refundRemaininghandler invocations against the new routing)forge test(running locally; will report)Follow-ups
Adding a CI gate that builds with the default profile and asserts every facet ≤ 24576 — anvil's
--disable-code-size-limitmasks regressions locally, and this PR is what we needed it to catch in the first place. Tracking separately so this fix can land first.