feat(web3): #2399 add an optional caller address to read-contract - #2413
feat(web3): #2399 add an optional caller address to read-contract#2413bongbongcrypto wants to merge 2 commits into
Conversation
…tract The eth_call this action makes carries no caller, so it goes out as address(0). For a contract whose answer depends on who is asking that is not a neutral read, and the answer can be the opposite of the true one. OptimismPortal2 reveals a failing withdrawal target only to address(1), and the note at protocols/chronicle.ts:15-17 records the same property from the other direction: a toll-gated Scribe feed reads for address(0) but not for an unauthed caller. Read Contract gains one optional config field, callerAddress. It is threaded through ReadContractRequest to the EVM adapter, which appends it as ethers call overrides, the way the write path at evm.ts:159 already does for its preflight staticCall. Empty or whitespace-only means absent, the rule functionArgs already uses, so a caller fed by a template that renders to nothing leaves the call unchanged rather than failing it. A present value that is not an address is refused before the chain call as a USER error, and it is not a destinationError: it is payload, like the arguments, so failOnError softens it the way it softens an unparseable argument list rather than hard-failing it the way it hard-fails an invalid contract address. With the field unset no overrides object is built and nothing is appended, so the call is byte-identical to the one made before. Both branches of the isView ternary take it. Wiring only one would be a silent half-fix, and the case that motivated this is the staticCall branch. Eight of the thirteen new tests fail against staging without this change, including both branches of that ternary and the adapter case that asserts the eth_call payload itself. The Solana adapter ignores its request and is untouched; batch-read-contract is deliberately out of scope, because through Multicall3 the inner call's msg.sender is Multicall3 no matter what caller is set, so the field would answer wrongly there.
…ield is not Three amendments from the issue review. The absence rule had the two layers disagreeing. validateFieldValue early-returns valid for undefined, null and "" only, and rejects a whitespace-only value through its isAddressField branch, so the runtime half of "empty or whitespace-only means absent" was unreachable for that input and said the opposite of save time about it. Absent is now that same set, a whitespace-only value fails isAddress here the way it fails validation there, and null is mapped alongside undefined because that is what an MCP-authored config carries and isMissingRequiredValue treats it as missing. The value is still trimmed before it is read, which is the case a template actually produces - an address with whitespace around it - and that is now asserted rather than assumed. The help text says plainly that nothing is signed or sent from the address and that a write is never sent from it, with the warning pointed at the real risk: gating a transfer on an answer obtained as someone else. Scope is stated rather than implied. readContractCore is also called from app/api/execute/contract-call/route.ts, check-and-execute/route.ts twice and [...slug]/route.ts, each enumerating fields explicitly, so each will drop this one. The Direct Execution API is deliberately out of scope here; this change reaches the workflow surface only. Tests: 41 across the two suites this touches, 346 across the 24 contract/adapter/web3 suites, biome clean on the four lintable files. Five mutants went red: the absence test trimming again, null no longer absent, the value no longer trimmed, the address check dropped, and the caller never reaching the adapter. Rebased onto staging. read-contract-core.ts is also touched by KeeperHub#2382, so whichever lands first the other rebases.
About the
|
suisuss
left a comment
suisuss
left a comment
There was a problem hiding this comment.
What this changes
callerAddress?: string on ReadContractRequest (lib/web3/chain-adapter/types.ts:46) and on ReadContractCoreInput (plugins/web3/steps/read-contract-core.ts:46); a validation block at :147-161 that normalises empty to absent, trims, and refuses a malformed value as USER before any provider is resolved; conditional forwarding at :346; the overrides spread into both arms at lib/web3/chain-adapter/evm.ts:265-271; and a template-input field with isAddressField: true at plugins/web3/index.ts:895-903. batch-read-contract is untouched.
I verified the part most likely to be wrong against the repo's installed ethers 6.17.0 rather than reading it: from reaches the eth_call tx object on a zero-argument view, a one-argument view, a nonpayable staticCall, a zero-argument staticCall, and a call whose last real argument is a tuple object - the overrides are not folded into it. With the field unset the tx carries from: undefined and the call is argument-identical to staging. Both arms converge on one populateTransaction, and copyOverrides keeps from while rejecting to and data.
The validation divergence I raised is resolved the right way and for the right reason. template-input has no case in validateFieldValue's switch, so it reaches default: and the isAddressField branch at action-config.ts:463-468; isMissingRequiredValue covers exactly undefined | null | "", which is exactly what the runtime givenCaller === "" test covers. Whitespace-only is refused at both layers. And a malformed caller fails rather than silently falling back to a no-caller read, which is the correct direction for this feature.
Does it match the description
Matches.
Mechanical - actionable as-is
-
lib/abi/utils.ts:140-155andlib/abi/function-key.ts:35-41are now load-bearing in a way they were not before, and nothing pins that. With a bare overloaded name of arity N and N+1, a trailing overrides object makes both candidates match and ethers throwsambiguous function description- I reproduced it against 6.17.0 withdeposit(uint256)/deposit(uint256,address). It is unreachable today becauseresolveAbiFunctionrejects an ambiguous bare name andgetAbiFunctionKeyemits a qualified signature whenever a name is shared. Worth a comment at the overrides construction saying the qualified-key guarantee is what makes it safe. -
read-contract-core.ts:148trims after the emptiness test, so a template rendering to" "is a hardUSERerror while one rendering to""means no caller. Deliberate and I agree with it; worth one line in the help tip, since incidental whitespace in a template decides between the two. -
action-config.ts:463validates with/^0x[0-9a-fA-F]{40}$/whileread-contract-core.ts:149usesethers.isAddress, which enforces EIP-55 on mixed case. An author who hand-edits one hex character's case saves clean and then fails every run. Pre-existing across several fields, not introduced here. -
docs/plugins/web3.md:83still lists the Read Contract inputs as "Network, Contract Address, ABI, Function, Function Arguments". Not covered bycheck:api-docs, which indexesdocs/apionly, so it will rot quietly. -
Two assertions exist only against the mock: the
staticCallbranch with overrides, and the zero-argument case, which is where ethers' arity heuristic is actually doing work. I verified both by hand against real ethers, so the risk is negligible - buttests/unit/evm-chain-adapter-read.test.tsis the file that could pin them, and a future ethers bump is exactly what would break them silently.
With the team
Nothing.
Verdict
Approve - the field reaches the wire on both branches, the unset path is byte-identical to today, and the save-time and runtime rules agree.
Two notes rather than findings. The five enumerating call sites - contract-call/route.ts:99, check-and-execute/route.ts:175 and :436, [...slug]/route.ts:206, protocol-read.ts:142 - all drop the field, which matches the scope we agreed; none of them accepts it from a request body either, so nothing is accepted-then-ignored. And #2382 does conflict, in the ReadContractCoreInput type block: it widens functionArgs with a comment on the line your callerAddress follows. I reconstructed the merge - one textual conflict, no semantic one, and the resolution is to keep both lines. Whichever lands second takes it.
2 participants
Closes #2399.
What this changes
web3/read-contractgains an optionalcallerAddress. Unset, the payload is byte-identical to today: no overrides object is constructed and theeth_callcarries nofromkey. Set, it becomes{ from }on the call, so a contract that answers by caller can be asked as the address that matters.ReadContractRequest(lib/web3/chain-adapter/types.ts) gainscallerAddress?: string.EvmChainAdapter.readContract(evm.ts) appends the overrides object when it is present and appends nothing when it is not, on both the view and thestaticCallbranch.read-contract-core.tsvalidates and forwards it, andplugins/web3/index.tsadds the config field betweenfunctionArgsandreadFailOnError.The three amendments from the issue
Absence rule. You were right that the two layers disagreed.
validateFieldValue(lib/workflow/validation/action-config.ts) early-returns valid forundefined,nulland""only, and rejects a whitespace-only value through itsisAddressFieldbranch, so "empty or whitespace-only means absent" was unreachable for that input and contradicted save time about it. Absent is now that same set. A whitespace-only value failsisAddresshere the way it fails validation there.nullis mapped alongsideundefined, because that is what an MCP-authored config carries andisMissingRequiredValuetreats it as missing, so such a workflow persists and then has to run. The value is still trimmed before it is read - an address with whitespace around it is what a template actually produces - and that case is asserted rather than assumed.Help text. It now says nothing is signed or sent from the address and that a write is never sent from it, with the warning pointed where you put it: gating a transfer on an answer obtained as someone else.
Scope. Stated rather than implied.
readContractCoreis also called fromapp/api/execute/contract-call/route.ts:99,check-and-execute/route.ts:175and:436, and[...slug]/route.ts:206, each enumerating fields explicitly, so each drops this one. The Direct Execution API is deliberately out of scope; this reaches the workflow surface only. Say if you want it there and I will add it.The design fork
Taking your reading: optional, defaulting to unset. Defaulting to the org wallet would change the answer every existing read gets, and a silent change of answer is the thing this issue is about. An explicit opt-in leaves
address(0)as the default caller, which is not neutral, but it is the default that is already there rather than a new one. Happy to be argued out of it.Out of scope, and worse than what I filed
Recording your find so it is not rediscovered:
check-and-execute/route.ts:175-182callsreadContractCorewithresolvedWriteAbi- thestaticCallbranch - to decide whether to broadcast. That is a write preflight with no caller, in a route that elsewhere callssimulateContractCall, which resolves the org wallet and sends a realfrom(lib/execute/simulate.ts:675,684). Two preflight styles in one file, one of them answering asaddress(0).batch-read-contractstays excluded, and your Multicall3 run is the sharper reason: withfrom=0x00...01aggregate3returnssuccess:false, returnData:0xab581036, so the portal's estimation branch keys ontx.origin. A caller field there would appear to work for this case and be wrong for everymsg.sender-gated contract.Checks
41 tests across
read-contract-coreandevm-chain-adapter-read; 346 across the 24 contract/adapter/web3 suites; biome clean on the four lintable files (plugins/is excluded bybiome.jsonc).Five mutants were planted and each went red: the absence test trimming again,
nullno longer absent, the value no longer trimmed, theisAddresscheck dropped, and the caller never reaching the adapter.Rebased onto staging.
read-contract-core.tsis also touched by #2382, so whichever lands first the other rebases.