feat(tipping): Nodle Tipping contract (v1 deployed, v2 on branch) by Douglasacost · Pull Request #134 · NodleCode/rollup

Douglasacost · 2026-10-27T17:17:46Z

What

src/tipping/: an immutable contract that lets anyone tip an NFT post or an account with any ERC-20 on zkSync Era, consumed by the Nodle feed indexer (NodleCode/nft-feed-indexer). This branch carries two generations of the contract; both are documented in nft-feed-indexer docs/specs/2026-10-27-tipping-authorization.md.

v1, deployed and live

Post tips carry an EIP-712 TipAuthorization signed by the feed API, which resolves the recipient off-chain; the contract verifies the signature and pays. Pausable, rotatable signer, fee ≤ 10 % sent per tip.

Chain	Address	Block	Admin	Signer
zkSync Era mainnet (324)	`0xB061fCeB3DFD851fa2240d90124b4deA60fc2405`	72662390	Safe `0x5e097AC1BCF81E7Ff2657045F72cAa6cF06486C9` (2 of 4)	`0xfe74C8C3f3F1ca3D4b523fd1AE7A3d82dbAc5eCe`
zkSync Sepolia (300)	`0x9741565272C7B29574c88ed2eBDF15BFE9C04612`	8761218	`0xc1f2…cd46`	`0xc1f2…cd46`

Both source-verified. First real mainnet tip 2026-10-28 (tx 0xa0d6afb7…3701). The cash apps (4.0.21) integrate v1.

v2, this branch's current code, deployed 2026-10-29 (final, after review)

Chain	Address	Block	Admin
zkSync Era mainnet (324)	`0xEc95147d72a3F0d111226831ee1dc92F68cEA8b5`	72673127	Safe `0x5e097AC1…86C9`
zkSync Sepolia (300)	`0x700e05CE499edF1C1Fd0a89134350Ba8a24dF1E3`	8768534	`0xc1f2…cd46`

Both source-verified; Sepolia acceptance green (nft-feed-indexer#43, TIP_VERSION=2). Earlier v2 deployments (0x1809…bDA4, 0xA7cD…6C63, 0xcC43…7E9b) predate the review fixes and are unused. The indexer indexes v1 and v2 and advertises v1 until TIP_CONTRACT_VERSION=2 (nft-feed-indexer#38, #44).

Review decisions of 2026-10-28/29:

No pause. The contract holds no funds; an admin power to stop tipping protects nobody.
No backend signature. tip(collection, standard, tokenId, recipient, token, amount): the client names the recipient and the contract verifies it: ERC-721 → ownerOf(tokenId); ERC-1155 (the collection must declare ERC-1155 via ERC-165) → the recipient holds the collection's OWNER_ROLE (Nodle user collections grant it to the creator), or is its Ownable owner(), or its ERC-2981 receiver for the id; ids with zero tracked supply are invalid. isValidTipRecipient() view. Nothing is enumerated on-chain, only checked.
Fees accrue in the contract. withdrawFees(token) (anyone) sends the whole balance of that token to the fee recipient; stray tokens are recoverable the same way.
Any ERC-20, any NFT, any account; the indexer counts only supported tokens. tipAccount kept (owner decision).

Invariant unchanged across versions: transferFrom only ever with from == msg.sender. Admin (DEFAULT_ADMIN_ROLE, the Safe) can only set the fee and the fee recipient.

Tooling in this branch

script/DeployTippingZkSync.s.sol (constructor: admin only for v2) and ops/deploy_tipping_zksync.sh <testnet|mainnet> [--broadcast], which moves the L1-only swarm files aside because zksolc cannot compile them.
ops/verify_zksync_contracts.py now inlines Foundry remappings; the explorer API rejects settings.remappings since 2026-10.
CI: coverage gate ≥ 95 % on src/tipping (.github/workflows/checks.yml).

Review

Two reviews of v1 on 2026-10-28 (one independent): no critical or high findings. The medium finding (spoofable AccountTipped with a worthless token) is handled in the indexer, which counts only supported tokens. The lows (tipping address(this), admin renounce, fee change between sign and execute, fee-on-transfer tokens) were accepted; v2's fee accrual also closes the address(this) one.

Tests

31 Foundry tests on v2, 98 % line coverage on src/tipping, spell check clean. forge test --zksync crashes in this repo (foundry-zksync linker); EraVM behaviour was checked on the deployed v1 and will be on the v2 Sepolia deployment.

Rollout of v2

Deploy on Sepolia for the app teams, then mainnet with the Safe as admin. The indexer already indexes both versions and advertises one through TIP_CONTRACT_VERSION (nft-feed-indexer#38); v1 and POST /v1/tips/authorize stay until both apps ship v2 support.

…olution (local, not deployed)

…anity

…overage gate - test/tipping/TippingZkSyncFork.t.sol: The Turtle registers via OWNER_ROLE and resolves to its creator, a legacy ClickContentSign token resolves to its owner (skipped without ZKSYNC_RPC_URL) - LOOKUP_GAS checked on EraVM with eth_call gas limits against mainnet: heaviest lookup (royaltyInfo through the Turtle proxy) needs ~70k including node overhead, cap is 100k - script/DeployTippingZkSync.s.sol (constructor-only deploy) and script/SeedTippingZkSync.s.sol (idempotent allowlist + creator seed) - CI: coverage job includes test/tipping, 95% line gate on src/tipping

github-actions · 2026-10-27T17:27:03Z

LCOV of commit `0a78683` during checks #804

Summary coverage rate:
  lines......: 25.8% (1037 of 4024 lines)
  functions..: 27.7% (177 of 638 functions)
  branches...: 26.0% (187 of 719 branches)

Files changed coverage rate:
                                                    |Lines       |Functions  |Branches    
  Filename                                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================================
  script/DeployTippingZkSync.s.sol                  | 0.0%     13| 0.0%     1| 0.0%      2
  src/tipping/Tipping.sol                           |98.2%     57| 100%     8| 100%     22
  test/tipping/Tipping.t.sol                        |97.7%     43| 100%    19|50.0%      2

… NFT, account tips) Replaces the on-chain registry/allowlist with an EIP-712 authorization signed by a rotatable signer (the feed API). The contract knows nothing about tokens or NFTs: - tip(TipAuthorization, signature): tipper == msg.sender, signer, deadline, single-use nonce - tipAccount(token, to, amount): caller-named recipient, no signature - fee <= 10% starting at 0, admin-only setters, pause, ERC-1271 signers supported - 25 tests (95.7% line coverage), fuzz that transferFrom is only ever from the caller, fuzz that the digest binds every field - deploy script takes admin + signer; seed script and fork test removed

…e for the compile

… rejects settings.remappings The zkSync verification API (checked 2026-10-28) returns 400 "settings.remappings is not supported; imports must match sources keys exactly". Source keys are already project-relative, so rewriting @openzeppelin/... imports to their remapped paths and dropping the setting gives full verification again. Verified Tipping on mainnet and Sepolia with it.

The contract holds no funds and only moves the caller's own tokens, so an admin power to stop tipping protects nobody; signer rotation already covers a leaked key. Removes pause/unpause, the whenNotPaused guards and the Paused/Unpaused events. Takes effect at the next deployment: the live mainnet and Sepolia contracts are immutable and keep pause.

Review decisions 2026-10-28 (points 2 and 4): tip(collection, standard, tokenId, token, amount) pays ownerOf(tokenId) for ERC-721 and the ERC-2981 royalty receiver for ERC-1155 (after rejecting ids with zero tracked supply when the collection exposes totalSupply). Lookups are STATICCALLs behind try/catch: a hostile collection can only make the tip revert. Drops EIP-712 TipAuthorization, signer, nonces and deadlines; adds recipientOf() for clients. Errors: NotAContract, InvalidToken, NoRecipient. Admin keeps fee and fee recipient. 24 tests, 100 % line coverage on src/tipping. Takes effect at the v2 deployment.

Decision 2026-10-28: nothing can be enumerated from an ERC-1155 (no owner getter, roles are not enumerable), but a claim can be checked. tip(collection, standard, tokenId, recipient, token, amount) accepts the recipient when: ERC-721, ownerOf(tokenId) == recipient; ERC-1155, recipient holds the collection's OWNER_ROLE (Nodle collections grant it to the creator), or is its Ownable owner(), or its ERC-2981 receiver for the id. Supply-tracking editions reject ids with zero supply. isRecipient() view for clients and the API. Replaces recipientOf() and NoRecipient with NotRecipient(collection, tokenId, recipient).

… them to the fee recipient Review point 3 (2026-10-28). The fee share of a tip is pulled into this contract instead of the fee recipient; anyone may call withdrawFees(token), which sends the contract's whole balance of that token to feeRecipient (so fee-on-transfer rounding and stray tokens are handled too). The allowance invariant is unchanged: transferFrom only from msg.sender.

aliXsed · 2026-10-29T03:54:01Z

+    }
+
+    /// @inheritdoc ITipping
+    function tipAccount(address token, address to, uint256 amount) external nonReentrant {


I don't think we need this, I suggest this contract serves only one purpose of tipping nfts. And as we discussed even narrower than that tipping only our own nfts. However to enable checking if the contract is one of ours, you would need to upgrade the factory. However the value we's get is thin here.

Keeping tipAccount for now (owner decision 2026-10-29): the cash apps 4.0.21 already integrate account tips from profiles. We can revisit once v2 is the only version in use. Agreed that "only our own NFTs" would need the factory to expose its creations, and that the value is thin for now.

aliXsed · 2026-10-29T04:02:04Z

+ *         Nodle feed indexer consumes it.
+ */
+interface ITipping {
+    enum Standard {


I'd suggest rename to NFTStandard

Done: NFTStandard.

aliXsed · 2026-10-29T04:08:27Z

+    }
+
+    /// @inheritdoc ITipping
+    function isRecipient(address collection, Standard standard, uint256 tokenId, address recipient)


I suggest renaming to isValidTipRecipient and mark it public

Done: isValidTipRecipient, public.

aliXsed · 2026-10-29T04:09:20Z

+    ///      supply, when exposed, must be non-zero) and `recipient` must hold OWNER_ROLE, be the
+    ///      Ownable owner, or be the ERC-2981 receiver for the id. Reverts for non-contracts and
+    ///      invalid tokens; returns false for a wrong recipient.
+    function _isRecipient(address collection, Standard standard, uint256 tokenId, address recipient)


I suggest removing this function and putting it's body entirely in isValidTipRecipient

Done: the internal helper is gone, its body lives in isValidTipRecipient.

aliXsed · 2026-10-29T04:13:19Z

+
+    /// @dev Shared guards and transfers. Only ever pulls from msg.sender: the property the
+    ///      immutability argument rests on. Returns the fee taken.
+    function _pay(address token, address to, uint256 amount) internal returns (uint256 fee) {


Suggest renaming to _transferTipWithFee

Done: _transferTipWithFee.

…ublic (internal check folded in) Addresses aliXsed's review: enum Standard → NFTStandard; isRecipient → isValidTipRecipient, public, with the former internal _isRecipient body inlined. tipAccount stays for now (owner decision 2026-10-29). ABI change (view function name only; tip() unchanged): redeploy before the apps integrate v2.

…ce delta Review rename. The invariant fuzz asserted a zero balance after the tip, which fails when the fuzzed tipper is ALICE (funded in setUp); it now asserts before - amount.

Copilot

🟡 Changes recommended

Unresolved critical flaws permit bypassing ERC-721 recipient checks and broadcasting mainnet deployments without confirmation, while verifier defects break contract verification.

4 open issues

What changed in this PR

Adds immutable Nodle Tipping v2 for ERC-20 tips to NFT posts and accounts on zkSync Era, with events consumed by the feed indexer.

Changes:

Replaces backend authorization with on-chain recipient checks and adds accrued-fee withdrawals.
Adds deployment tooling and import-remapping support for source verification.
Adds Foundry tests and a 95% tipping line-coverage gate.

File	Description
`test/tipping/Tipping.t.sol`	Tests recipient checks, transfers, fees, access controls, and caller-only spending.
`src/tipping/Tipping.sol`	Implements v2 tipping, recipient validation, and fee management.
`src/tipping/ITipping.sol`	Defines the v2 interface, events, and errors.
`script/DeployTippingZkSync.s.sol`	Deploys Tipping with a configured admin.
`ops/verify_zksync_contracts.py`	Rewrites remapped imports before verification.
`ops/deploy_tipping_zksync.sh`	Wraps network deployment and temporarily excludes L1-only files.
`.github/workflows/checks.yml`	Includes tipping tests in coverage and enforces the threshold.

_{Review effort: Balanced}

Douglasacost · 2026-10-29T04:42:59Z

+set -euo pipefail
+cd "$(dirname "$0")/.."
+
+NETWORK="${1:-}"; BROADCAST="${2:-}"


Valid, fixed: the second argument is now validated (--broadcast or nothing), so the guard and the flag can no longer disagree.

Douglasacost · 2026-10-29T04:43:01Z

+        try IERC1155SupplyLike(collection).totalSupply(tokenId) returns (uint256 supply) {
+            if (supply == 0) revert InvalidToken(collection, tokenId);
+        } catch {
+            // No supply extension: nothing to check here.
+        }


Valid, fixed: the ERC-1155 path now requires supportsInterface(type(IERC1155).interfaceId) before any of the role/owner/royalty proofs; a role-bearing ERC-721 declared as ERC-1155 is refused. Regression test test_tip_erc721WithOwnerRolePassedAs1155IsRefused with a Nodle-style ERC-721 (OWNER_ROLE, no totalSupply(id)).

Douglasacost · 2026-10-29T04:43:02Z

+# broadcast/DeployTippingZkSync.s.sol/<chainId>/run-latest.json; verify afterwards with
+# python3 ops/verify_zksync_contracts.py --broadcast <that file>.


Valid, fixed: Tipping added to CONTRACT_SOURCE_MAP and DeployTippingZkSync.s.sol to BROADCAST_CONTRACT_SEQUENCE.

Douglasacost · 2026-10-29T04:43:04Z

+    pattern = re.compile(r'(import\s+.*?["\'])([^"\'.][^"\']*)(["\'].*)')
+    changed = 0
+    for src_key in list(std_json["sources"].keys()):
+        lines = std_json["sources"][src_key]["content"].split("\n")
+        out = []
+        for line in lines:
+            m = pattern.match(line)
+            if m:
+                prefix, path, suffix = m.groups()
+                for rp, target in pairs:
+                    if path.startswith(rp):
+                        new_path = target + path[len(rp):]
+                        if new_path in source_keys:
+                            line = prefix + new_path + suffix
+                            changed += 1
+                        break
+            out.append(line)
+        std_json["sources"][src_key]["content"] = "\n".join(out)


Valid, fixed: remappings are now rewritten per import statement with a whole-content regex that spans line breaks, so import {X} from\n "@openzeppelin/..." in the UserCollection sources resolves too.

- isValidTipRecipient: the ERC-1155 proofs apply only to collections that declare ERC-1155 support (ERC-165); a role-bearing ERC-721 passed as ERC-1155 no longer skips ownerOf (regression test with a Nodle-style ERC-721). - ops/deploy_tipping_zksync.sh: the second argument must be exactly --broadcast or omitted, so the mainnet confirmation guard and the broadcast flag cannot disagree. - ops/verify_zksync_contracts.py: Tipping in CONTRACT_SOURCE_MAP and DeployTippingZkSync in BROADCAST_CONTRACT_SEQUENCE; remappings are rewritten per import statement, including statements that span lines (UserCollection*.sol).

… (coverage gate)

Douglasacost added 4 commits October 22, 2026 17:05

feat(tipping): immutable Tipping contract with on-chain recipient res…

7a73cd9

…olution (local, not deployed)

fix(tipping): zero-amount guard, single registration, fee-recipient s…

0e8d8bd

…anity

test(tipping): full suite (31 tests, 97% line coverage), local only

f2354a9

Douglasacost changed the title ~~feat(tipping): immutable Tipping contract with on-chain recipient resolution~~ feat(tipping): agnostic signature-authorized tipping contract Oct 27, 2026

Douglasacost added 10 commits October 27, 2026 14:55

test(tipping): rename mock field for spell check

9f6438c

Merge branch 'main' into feat/tipping

6d7416b

ops(tipping): zksync deploy wrapper that moves the L1-only files asid…

9a9bfa7

…e for the compile

test(tipping): rename for the spell check

9850026

docs(tipping): contract title and description for v2

f3399d6

Douglasacost changed the title ~~feat(tipping): agnostic signature-authorized tipping contract~~ feat(tipping): Nodle Tipping contract (v1 deployed, v2 on branch) Oct 29, 2026

aliXsed reviewed Oct 29, 2026

View reviewed changes

Douglasacost added 2 commits October 29, 2026 00:17

refactor(tipping): _pay → _transferTipWithFee; fuzz asserts the balan…

73a6f93

…ce delta Review rename. The invariant fuzz asserted a zero balance after the tip, which fails when the fuzzed tipper is ALICE (funded in setUp); it now asserts before - amount.

aliXsed requested a balanced review from Copilot October 29, 2026 04:25

Copilot started reviewing on behalf of aliXsed October 29, 2026 04:26

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

aliXsed approved these changes Oct 29, 2026

View reviewed changes

test(tipping): cover the ERC-165 refusal and the no-proof fallthrough…

0a78683

… (coverage gate)

Douglasacost merged commit d58e4b4 into main Oct 29, 2026
3 checks passed

Douglasacost deleted the feat/tipping branch October 29, 2026 05:30

		# broadcast/DeployTippingZkSync.s.sol/<chainId>/run-latest.json; verify afterwards with
		# python3 ops/verify_zksync_contracts.py --broadcast <that file>.

Conversation

Douglasacost commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

v1, deployed and live

v2, this branch's current code, deployed 2026-10-29 (final, after review)

Tooling in this branch

Review

Tests

Rollout of v2

Uh oh!

github-actions Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LCOV of commit 0a78683 during checks #804

Uh oh!

aliXsed Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Douglasacost commented Oct 27, 2026 •

edited

Loading

github-actions Bot commented Oct 27, 2026 •

edited

Loading

LCOV of commit `0a78683` during checks #804

aliXsed Oct 29, 2026 •

edited

Loading