3.3: migrate signBalanceRead to the EIP-712 chainId domain#18
Open
ameya-deshmukh wants to merge 2 commits into
Open
3.3: migrate signBalanceRead to the EIP-712 chainId domain#18ameya-deshmukh wants to merge 2 commits into
ameya-deshmukh wants to merge 2 commits into
Conversation
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.
Downstream of seismic monorepo #209 (Zellic finding 3.3). Contract PR #209 changed
SRC20.balanceOfSigned's signature scheme from a personal-sign message to an EIP-712 typed-data digest bound tochainId. This repo's TS signer still built the old scheme, so signed reads would recover the wrong address and revert onrequire(signer == owner). This PR migrates the signer.Scheme change (must match contract #209 byte-for-byte)
keccak256(abi.encodePacked("SRC20_BALANCE_READ", owner, expiry)).verifyingContract, nosalt):Changes
packages/batch-read-ts/src/util/signature.ts—signBalanceReadrewritten to use viemsignTypedDataproducing the EIP-712 digest above.signBalanceReadnow takes a requiredchainIdargument (the domain is chain-bound). Breaking for any external caller.packages/batch-read-ts/src/index.ts— the sole in-repo caller now passeschain.id.Test plan (test-first)
test:commit addssignature.test.tsreconstructing the EIP-712 digest by formula for a fixed(owner, expiry, chainId), signing a known key, and assertingrecoverAddress(digest, sig) == owner. Fails on the old personal-sign scheme (recovers a different address), passes on the new one.bun testgreen;prettier --checkclean.Note — stale vendored contract (not fixed here)
packages/contracts/dependencies/seismic-std-lib-0.1.3/SRC20.sol:253still implements the old personal-sign scheme. That belongs to the upstreamseismic-std-libversion bump (the std-lib tracker's 3.3 propagation), not this TS-signer migration. Flagging so it isn't missed — deploying this signer against the stale vendored contract would revert until the dependency is bumped to the #209 release.