feat: follow proxies and use Sourcify as the only source of reference ABIs - #349
Open
marcocastignoli wants to merge 28 commits into
Open
marcocastignoli wants to merge 28 commits into
marcocastignoli wants to merge 28 commits into
Conversation
Request proxyResolution along with the ABI and append the ABIs of every implementation (following nested proxies) to the ABIs of the proxy. An implementation that is not verified on Sourcify is a failure. Linters now validate against the merged ABIs instead of guessing proxies from function names and skipping validation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hains Remove the Etherscan fallback when fetching reference ABIs: a contract that is not verified on Sourcify is a failure. Reference ABIs come from Sourcify, so link to the contract on the Sourcify repository in lint messages and use the Sourcify chain list instead of the Etherscan one. The Etherscan transport stays, it is still needed for descriptors whose ABI is an Etherscan URL. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sourcify responses have no caching headers, so they were never stored in the HTTP cache and every lint run fetched every contract again. Force caching (responses still expire with the storage TTL, 7 days). Also memoize get_contract_abis, as reference ABIs of a deployment are fetched by several linters during the same run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sourcify computes proxy resolution at request time and reports a failure in the response body with HTTP 200. Such a response was parsed as "not a proxy", validating the descriptor against the proxy ABI alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Raise ContractNotVerifiedError, ProxyImplementationNotVerifiedError and ChainNotSupportedError from the client, reading Sourcify's customCode on HTTP 400, and give each its own title in the linters. "Could not fetch ABI" is kept for transient failures. Remove the unreachable "no ABI" branches, and document the new checks in place of the removed proxy one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The URL no longer depends on the chain list, and the lookup only added a network failure mode that the linters did not handle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Force caching per request, only for Sourcify responses, instead of for every host: other responses follow their caching headers again. Shorten the cache TTL to one hour, as it is now the freshness window for proxy resolution, and add ERC7730_NO_CACHE to disable the cache. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The test only stubbed the proxy lookup and fetched the implementation from the live API. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The v2 classifier swallowed every reference ABI failure. Trace which deployment was skipped and why, at debug level since the display fields linter already reports the failure itself. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
feat: follow proxies and use Sourcify as the only ABI source
ledger-asset-dapps has a nested submodule with an SSH URL, which makes pip fail to install the library from git on machines without a GitHub SSH key. Mark both test registries update=none so a recursive update skips them, and check them out explicitly in CI and the developer docs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
build: skip test registries on recursive submodule update
When SOURCIFY_TOKEN is set, add X-Sourcify-Token to every request to sourcify.dev, so that a caller with a token can be exempted from rate limiting. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
feat(client): send a token header on Sourcify requests
With the flag, a contract, proxy implementation or chain that Sourcify does not know is reported as an error instead of a warning, so the lint fails. Transient fetch failures stay warnings. Off by default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
feat(lint): add --require-verified
Fetch the reference ABI of every deployment instead of stopping at the first one that succeeds. Deployments exposing the same functions are validated once; when they differ, a warning lists the groups and each distinct ABI is validated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Compare the functions as seen by a caller (name, parameters, state mutability) rather than the full ABI model, so that deployments differing only in compiler details such as internalType are validated once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…file The registry file can change with the daily submodule update; the tests depended on its two deployments and parameter names. Share the linting helper between the require-verified and all-deployments tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
feat(lint): validate display fields against every deployment
…rified With --require-verified, a reference ABI that could not be fetched (rate limit, 5xx, timeout, proxy resolution failure) is now an error instead of a warning, in both the v1 ABI linter and the v2 display fields linter. Before, a strict run could skip a deployment and still exit 0. Without the flag it stays a warning. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ire-verified The two lint_all entry points still described the flag as covering unverified contracts only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(lint): fail on any reference ABI fetch failure under --require-verified
MixedCaseAddress accepted any 40 hex digits in any letter case, so a mixed-case address with a wrong checksum passed the lint. Such an address is most likely a typo or a corrupted copy, and the Sourcify API rejects it. A mixed-case address must now equal its EIP-55 form; the error shows the expected form. An address in lowercase only (or uppercase only) carries no checksum and stays accepted, so no descriptor of the registry changes: 1801 deployment addresses, 188 constants and 50 parameters on master pass. The chain-specific EIP-1191 checksum is not supported. The check covers every field typed MixedCaseAddress: deployments, verifyingContract, token, nativeCurrencyAddress, senderAddress, callee, spender, collection. A value that comes from $.metadata.constants is not seen by the input model, so the resolver validates it with the same rules (resolved_address), for constants and for path-or-value fields of type address. assert_not_address now matches the shape only, so an address with a wrong checksum written in place of a path still gets the "use a constant" error. In the v1 senderAddress resolver, a constant that resolved to None fell through to the exception; it now resolves to None. Two v1 fixtures used made-up mixed-case addresses; they are lowercase now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Limit the resolver change to v2: the v1 resolver files are back to main, and the helper lives in convert/resolved/v2/address.py. The shared MixedCaseAddress type still reaches the v1 input model, so the two v1 fixtures keep their lowercase addresses. - Do not check a constant interoperableAddressName value as a 20 bytes address. The format has the address type family, but an ERC-7930 interoperable address is a longer binary value, so a valid value failed with "Invalid address". resolve_path_or_constant_value takes an evm_address flag; resolve_field_value clears it for this format. - Reject an address in uppercase only. EIP-55 does not define it as a form without checksum, and the type says "EIP-55 or lowercase". - One address shape: MixedCaseAddress uses ADDRESS_PATTERN, and assert_not_address uses fullmatch. - resolved_addresses replaces the three copies of the list loop. - Tests: the v2 InputDeployment, a field value written as a literal and as a constant (addressName, tokenTicker), the callee, spender, collection and senderAddress parameters from a constant, and the interoperableAddressName value. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fix(model): reject an address with a wrong EIP-55 checksum
This branch has not been deployed
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
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.
What
Reference ABIs are fetched from Sourcify only, and proxies are followed. This is the work we discussed for the Sourcify fork of the library; the commits are separate so you can take the proxy following without the Etherscan removal.
proxyResolutionand merges the ABI of every implementation into the proxy's ABI, following nested proxies and diamonds (checked on the LI.FI diamond, 40 facets). The "likely to be a proxy, validation skipped" shortcut is removed from both linters, so proxies are validated against the merged ABIs. A proxy resolution error, or an implementation that is not verified on Sourcify, is a failure for that deployment rather than a silent fallback to the proxy's own ABI.ETHERSCAN_API_KEYdocumentation forlintandgenerate. Supported chains come from Sourcify, and lint messages link torepo.sourcify.dev. The Etherscan transport stays for descriptors whoseabifield is an Etherscan URL.ContractNotVerifiedError,ProxyImplementationNotVerifiedErrorandChainNotSupportedErrorreplace the single "Could not fetch ABI" case, each with its own lint title (documented indocs/pages/lint.md). "Could not fetch ABI" is kept for transient failures, and the classifier now traces the deployments it skips.get_contract_abisis memoized within a run, andERC7730_NO_CACHE=1disables the cache.SOURCIFY_TOKENset,X-Sourcify-Tokenis sent on Sourcify requests, for callers exempted from rate limiting.update = noneon the two registry submodules, with an explicit--checkoutin CI and the developer docs, sopip installfrom git works without an SSH key (ledger-asset-dappshas a nested submodule with an SSH URL).Effect on the registry
erc7730 lintover the clear signing registry, before and after:"Missing display format" goes up because the merged ABIs include the proxy admin functions. The 7 remaining fetch failures are contracts verified nowhere, Sei precompiles, an unverified implementation, and a chain Sourcify does not support.
Proxy patterns Sourcify does not resolve will keep failing: Celo core Proxy, Aragon AppProxyUpgradeable (Lido stETH), StarkGate, Kiln VaultBeaconProxy, Polygon ValidatorShareProxy.
Tests
With
--run-v1 --run-integration: 2558 passed, 2 skipped, 2 failed. The 2 failures are real descriptor bugs that the proxy shortcut was hiding:lido/calldata-stETH-L2.jsonandlido/calldata-wstETH-L2.jsonuse paths like#.amountwhile the implementation ABI names the parametersamount_,recipient_and so on. They need a fix inclear-signing-erc7730-registry.🤖 Generated with Claude Code