diff --git a/README.md b/README.md index 98318c4..0f77f11 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ Arc is an open EVM-compatible layer 1 built on [Malachite](https://github.com/ci - 🚀 **[Execution](crates/node/README.md)** - Execution binary and configuration - 🗳️ **[Consensus](crates/malachite-app/README.md)** - Consensus binary and configuration +- 📝 **[Transaction memos](docs/transaction-memos.md)** - Memo contract ABI, gas estimation, and common pitfalls - More: see Arc [developer docs](https://docs.arc.network/arc/concepts/welcome-to-arc) for guides, APIs, and specs ## Install and Run a Node diff --git a/contracts/README.md b/contracts/README.md index d158c99..9074e7e 100644 --- a/contracts/README.md +++ b/contracts/README.md @@ -2,6 +2,16 @@ This directory contains Solidity contracts and tests for the Arc project, built with Foundry. +## Memo contract + +- Source: [`src/memo/Memo.sol`](src/memo/Memo.sol) +- Interface: [`src/memo/IMemo.sol`](src/memo/IMemo.sol) +- Developer guide (ABI, `eth_estimateGas`, pitfalls): [docs/transaction-memos.md](../docs/transaction-memos.md) + +Canonical entry point: `memo(address,bytes,bytes32,bytes)`. There is no +`callWithMemo` function — wrong selectors cause empty reverts during gas +estimation (see issue [#189](https://github.com/circlefin/arc-node/issues/189)). + ## Compiler choice for genesis-deployed contracts **Forge is the canonical compiler** for every CREATE2-deployed contract in Arc genesis diff --git a/contracts/src/memo/IMemo.sol b/contracts/src/memo/IMemo.sol index 55333ba..8bae630 100644 --- a/contracts/src/memo/IMemo.sol +++ b/contracts/src/memo/IMemo.sol @@ -25,6 +25,16 @@ pragma solidity ^0.8.29; /// /// Target-contract reverts (callFrom returning `success=false`) are /// re-raised as {MemoFailed} with the target's revert bytes. +/// +/// @dev Canonical entry point is {memo} with signature +/// `memo(address,bytes,bytes32,bytes)`. There is no `callWithMemo` function. +/// Calling a wrong selector reverts with empty data and will also make +/// `eth_estimateGas` / `eth_call` fail — that is an ABI mismatch, not a +/// CallFrom simulation bug. Pass an EOA as `from` when estimating. +/// +/// @dev {memo} is state-changing (`memoIndex++`). STATICCALL into Memo is +/// rejected; ordinary `eth_call` / `eth_estimateGas` (non-static top-level +/// simulation) succeeds with the correct ABI. interface IMemo { /// @notice Thrown when the call via callFrom fails. error MemoFailed(bytes returnData); @@ -50,6 +60,6 @@ interface IMemo { /// @param target The address to call via the precompile. /// @param data The calldata to forward to the target. /// @param memoId A caller-supplied identifier for the memo. - /// @param memoData Arbitrary memo bytes attached to the subcall. + /// @param memoData Arbitrary memo bytes attached to the subcall (not `string`). function memo(address target, bytes calldata data, bytes32 memoId, bytes calldata memoData) external; } diff --git a/contracts/src/memo/Memo.sol b/contracts/src/memo/Memo.sol index da6b11b..42fb5bf 100644 --- a/contracts/src/memo/Memo.sol +++ b/contracts/src/memo/Memo.sol @@ -27,6 +27,9 @@ import {IMemo} from "./IMemo.sol"; * The callFrom precompile enforces its own allowlist — this contract does not add access control. * @dev EOA-only: contract callers hit the callFrom sender-spoofing constraint * and the call reverts without raising {MemoFailed}. See {IMemo}. + * @dev Entry point is {memo} only (`memo(address,bytes,bytes32,bytes)`). There is + * no `callWithMemo`. Wrong selectors cause empty reverts during + * `eth_estimateGas` / `eth_call` and on-chain execution. See {IMemo}. */ contract Memo is IMemo { /// @inheritdoc IMemo diff --git a/contracts/test/memo/Memo.t.sol b/contracts/test/memo/Memo.t.sol index 8d3b89b..b9e3b4d 100644 --- a/contracts/test/memo/Memo.t.sol +++ b/contracts/test/memo/Memo.t.sol @@ -19,6 +19,7 @@ pragma solidity ^0.8.29; import {Test} from "forge-std/Test.sol"; import {Memo as MemoContract} from "../../src/memo/Memo.sol"; +import {IMemo} from "../../src/memo/IMemo.sol"; import {Addresses} from "../../scripts/Addresses.sol"; /// @dev Mock that simulates the callFrom precompile by forwarding the call to the target. @@ -88,6 +89,17 @@ contract MemoTest is Test { assertEq(memoContract.memoIndex(), 1); } + /// @dev Regression guard for the #189 pitfall class: only the `memo` selector + /// is implemented. A non-existent `callWithMemo` selector must not + /// accidentally dispatch into `memo` (would require matching 4-byte + /// selector, which it does not). + function test_callWithMemo_selector_is_not_memo() public pure { + bytes4 memoSel = IMemo.memo.selector; + // keccak256("callWithMemo(address,bytes,bytes32,string)") first 4 bytes + bytes4 callWithMemoSel = bytes4(keccak256("callWithMemo(address,bytes,bytes32,string)")); + assertTrue(memoSel != callWithMemoSel, "callWithMemo must not share memo selector"); + } + function test_memo_memoIndexIncrements() public { bytes memory data = abi.encodeCall(MockTarget.setValue, (1)); bytes32 memoId = keccak256("memo"); diff --git a/docs/transaction-memos.md b/docs/transaction-memos.md new file mode 100644 index 0000000..b03cdf1 --- /dev/null +++ b/docs/transaction-memos.md @@ -0,0 +1,144 @@ +# Transaction Memos + +How the predeployed `Memo` contract attaches metadata to a contract call on Arc +while preserving the original EOA as `msg.sender`. + +Official product docs: [Transaction memos](https://docs.arc.io/arc/concepts/transaction-memos). + +## Contract + +| Network | Address | +| --- | --- | +| Local genesis / testnet predeploy | `0x5294E9927c3306DcBaDb03fe70b92e01cCede505` | + +Source: [`contracts/src/memo/Memo.sol`](../contracts/src/memo/Memo.sol) + +## Canonical ABI + +The only entry point is: + +```solidity +function memo( + address target, + bytes calldata data, + bytes32 memoId, + bytes calldata memoData +) external; +``` + +| Parameter | Type | Description | +| --- | --- | --- | +| `target` | `address` | Contract to call (e.g. USDC) | +| `data` | `bytes` | Calldata forwarded to `target` | +| `memoId` | `bytes32` | Application-defined identifier | +| `memoData` | `bytes` | Arbitrary memo payload (**not** `string`) | + +There is **no** `callWithMemo` function on this contract. + +## Gas estimation and `eth_call` (issue #189) + +`memo` is state-changing (`memoIndex++`) but ordinary top-level simulation is +non-static. With the **correct** ABI and an EOA as `from`: + +- `eth_estimateGas` succeeds +- `eth_call` succeeds (state changes are discarded after simulation) +- A real transaction using the estimated gas limit succeeds on-chain + +Regression coverage lives in `tests/localdev/subcall.test.ts` +(`eth_estimateGas succeeds for memo...`, `eth_call succeeds for memo...`, +`estimated gas is enough to execute memo transfer on-chain`). + +### Common pitfall: wrong function name or types + +Callers that use a non-existent signature such as: + +```text +callWithMemo(address,bytes,bytes32,string) +``` + +hit a selector mismatch. Solidity reverts with **empty** return data. Libraries +then report `execution reverted` during `eth_estimateGas` / `eth_call`, which is +easy to misread as a CallFrom or node simulation bug. + +**Fix:** use `memo(address,bytes,bytes32,bytes)` and encode the memo as `bytes` +(e.g. `toHex('invoice-123')` / `toBytes(...)` in viem), not as a Solidity +`string`. + +### Other guardrails (expected reverts) + +These are intentional, not estimation bugs: + +| Pattern | Result | +| --- | --- | +| Contract wallet / intermediary calls `memo` | Revert (EOA-only / no sender spoofing) | +| `STATICCALL` into `Memo` | Revert (state change in static context) | +| EOA calls `CallFrom` precompile directly | `unauthorized caller` | +| Inner target reverts | Outer tx reverts with `MemoFailed(bytes)` | + +## Example: estimate then send (cast) + +```bash +MEMO=0x5294E9927c3306DcBaDb03fe70b92e01cCede505 +USDC=0x3600000000000000000000000000000000000000 +FROM= + +# Estimate (must pass --from) +cast estimate --from $FROM $MEMO 'memo(address,bytes,bytes32,bytes)' \ + $USDC \ + "$(cast calldata 'transfer(address,uint256)' $FROM 2)" \ + "$(cast to-uint256 1)" \ + "0x$(printf 'test memo' | xxd -p -c 256)" \ + --rpc-url http://localhost:8545 + +# eth_call simulation +cast call --from $FROM $MEMO 'memo(address,bytes,bytes32,bytes)' \ + $USDC \ + "$(cast calldata 'transfer(address,uint256)' $FROM 2)" \ + "$(cast to-uint256 1)" \ + "0x$(printf 'test memo' | xxd -p -c 256)" \ + --rpc-url http://localhost:8545 +``` + +## Example: viem + +```typescript +import { encodeFunctionData, erc20Abi, toBytes, toHex } from 'viem' + +const innerCalldata = encodeFunctionData({ + abi: erc20Abi, + functionName: 'transfer', + args: [recipient, amount], +}) + +await walletClient.writeContract({ + address: MEMO_ADDRESS, + abi: [ + { + name: 'memo', + type: 'function', + stateMutability: 'nonpayable', + inputs: [ + { name: 'target', type: 'address' }, + { name: 'data', type: 'bytes' }, + { name: 'memoId', type: 'bytes32' }, + { name: 'memoData', type: 'bytes' }, + ], + outputs: [], + }, + ], + functionName: 'memo', + args: [ + USDC_ADDRESS, + innerCalldata, + toHex(1n, { size: 32 }), + toBytes('Invoice INV-2026-001'), + ], + // Explicit gas is optional when the ABI is correct; estimateGas works. +}) +``` + +## Related + +- Local tests: `tests/localdev/subcall.test.ts` +- CallFrom precompile: `crates/precompiles/src/call_from.rs` +- Multicall3From (also uses CallFrom): `contracts/src/batch/Multicall3From.sol` diff --git a/tests/localdev/subcall.test.ts b/tests/localdev/subcall.test.ts index 8fc2485..8739154 100644 --- a/tests/localdev/subcall.test.ts +++ b/tests/localdev/subcall.test.ts @@ -409,6 +409,113 @@ describe('Memo', () => { }) }) + // Regression for issue #189: eth_estimateGas / eth_call must succeed for the + // canonical `memo(address,bytes,bytes32,bytes)` entry point when `from` is an + // EOA. The original report used a non-existent `callWithMemo(...string)` + // signature, which reverts with empty data during estimation and looks like a + // node/precompile simulation bug. + it('eth_estimateGas succeeds for memo(address,bytes,bytes32,bytes)', async () => { + const { client, sender, receiver } = await clients() + + const amount = USDC.parseUnits('0.001') + const transferData = encodeUSDCTransfer(receiver.account.address, amount) + const callData = encodeMemo(USDC.address, transferData, keccak256(toHex('estimate-memo')), toHex('estimate ok')) + + const gas = await client.estimateGas({ + account: sender.account.address, + to: memoAddress, + data: callData, + }) + + expect(gas).to.be.a('bigint') + expect(gas).to.be.greaterThan(21_000n) + }) + + // eth_call uses the same non-static simulation path as estimateGas; both must + // accept state-changing memo() simulation (memoIndex++ is discarded after the call). + it('eth_call succeeds for memo(address,bytes,bytes32,bytes)', async () => { + const { client, sender, receiver } = await clients() + + const amount = USDC.parseUnits('0.001') + const transferData = encodeUSDCTransfer(receiver.account.address, amount) + const callData = encodeMemo(USDC.address, transferData, keccak256(toHex('eth-call-memo')), toHex('call ok')) + + // Succeeds without throwing (return data is empty for non-view memo()). + await client.call({ + account: sender.account.address, + to: memoAddress, + data: callData, + }) + }) + + // Wrong / outdated ABI (callWithMemo with string memo) must not match memo(). + // Empty revert data is expected and is NOT a CallFrom simulation failure. + it('outdated callWithMemo(string) selector reverts (issue #189 pitfall)', async () => { + const { client, sender, receiver } = await clients() + + const amount = USDC.parseUnits('0.001') + const transferData = encodeUSDCTransfer(receiver.account.address, amount) + + // Historical incorrect signature from issue #189: + // callWithMemo(address,bytes,bytes32,string) — never deployed on Memo. + const wrongAbi = parseAbi([ + 'function callWithMemo(address target, bytes data, bytes32 memoId, string memo) external returns (bool, bytes)', + ]) + const wrongData = encodeFunctionData({ + abi: wrongAbi, + functionName: 'callWithMemo', + args: [USDC.address, transferData, keccak256(toHex('wrong-abi')), 'test'], + }) + + await expect( + client.estimateGas({ + account: sender.account.address, + to: memoAddress, + data: wrongData, + }), + ).to.be.rejected + + await expect( + client.call({ + account: sender.account.address, + to: memoAddress, + data: wrongData, + }), + ).to.be.rejected + }) + + // Gas estimated for memo() must be sufficient to land a real transaction. + it('estimated gas is enough to execute memo transfer on-chain', async () => { + const { client, sender, receiver } = await clients() + + const amount = USDC.parseUnits('0.001') + const nativeAmount = USDC.toNative(amount) + const transferData = encodeUSDCTransfer(receiver.account.address, amount) + const callData = encodeMemo( + USDC.address, + transferData, + keccak256(toHex('estimate-then-send')), + toHex('send with estimate'), + ) + + const gas = await client.estimateGas({ + account: sender.account.address, + to: memoAddress, + data: callData, + }) + + const balances = await balancesSnapshot(client, { sender, receiver }) + const receipt = await sender + .sendTransaction({ to: memoAddress, data: callData, gas }) + .then(ReceiptVerifier.waitSuccess) + + expect(receipt.gasUsed).to.be.lessThanOrEqual(gas) + await balances + .increase({ receiver: nativeAmount }) + .decrease({ sender: nativeAmount + receipt.totalFee() }) + .verify() + }) + // EOA directly calling callFrom is not allowlisted — reverts with "unauthorized caller". // sender → callFrom(sender, receiver, "0x") → REVERT it('unauthorized direct call to callFrom precompile reverts', async () => {