Keeps eth_sign and makes its warning and the README say what it does, as settled on #409.
eth_sign was already signed as an EIP-191 personal message over its data, the same way personal_sign is. That is what the Ethereum JSON-RPC specification defines eth_sign to return, so the signing code is unchanged. The untrue parts were the warning on the signature prompt ("This site is requesting to sign a raw hash ... can be used to sign transactions that drain your funds") and the README line calling it raw-hash signing.
The warning on an eth_sign prompt now reads: "This site used the older eth_sign method. The data will be signed as a personal message."
README, SignApproval: the warning box line quotes that sentence and says eth_sign names the address first and the data second.
tests/backgroundApproval.test.js sends eth_sign with the address first and the data second, checks the approval carries them the right way round with the new warning, and checks the page receives the personal message signature over the data.
tests/ethSign.test.js boots the popup as the approval window for an eth_sign request, signs it with a known key, and checks the signature the sign screen sends back is the personal message signature over the data's bytes.
Judgement call: the sentence still appears in the existing red warning box, and the internal names dangerWarning and approve-sign-danger-warning are unchanged; only the text changed.
Model: opus-5-5
Keeps `eth_sign` and makes its warning and the README say what it does, as settled on https://git.eeqj.de/sneak/AutistMask/issues/409.
`eth_sign` was already signed as an EIP-191 personal message over its data, the same way `personal_sign` is. That is what the Ethereum JSON-RPC specification defines `eth_sign` to return, so the signing code is unchanged. The untrue parts were the warning on the signature prompt ("This site is requesting to sign a raw hash ... can be used to sign transactions that drain your funds") and the README line calling it raw-hash signing.
- The warning on an `eth_sign` prompt now reads: "This site used the older eth_sign method. The data will be signed as a personal message."
- README, SignApproval: the warning box line quotes that sentence and says `eth_sign` names the address first and the data second.
- `tests/backgroundApproval.test.js` sends `eth_sign` with the address first and the data second, checks the approval carries them the right way round with the new warning, and checks the page receives the personal message signature over the data.
- `tests/ethSign.test.js` boots the popup as the approval window for an `eth_sign` request, signs it with a known key, and checks the signature the sign screen sends back is the personal message signature over the data's bytes.
Judgement call: the sentence still appears in the existing red warning box, and the internal names `dangerWarning` and `approve-sign-danger-warning` are unchanged; only the text changed.
Model: opus-5-5
tests/backgroundApproval.test.js:1588: the new eth_sign test makes the signature itself and hands it to the background, so the code that signs an eth_sign request, src/popup/views/approval.js:1133, is not run by any test, and no browser test sends eth_sign. The parameter order and the background's check of the signature are pinned; the signature the wallet produces is not, which the plan on #409 asks for. If the sign screen signed an eth_sign request any other way, every eth_sign would be refused and every test would still pass. Acceptable: a test that opens the sign screen for an eth_sign request, signs with a known key (with the password step set up as tests/walletDefects.test.js does), and checks that the signature the screen sends back is the personal message signature over the data's bytes; or an eth_sign round trip in the browser suites next to the personal_sign one.
Model: opus-5-5
**FAIL**
1. `tests/backgroundApproval.test.js:1588`: the new `eth_sign` test makes the signature itself and hands it to the background, so the code that signs an `eth_sign` request, `src/popup/views/approval.js:1133`, is not run by any test, and no browser test sends `eth_sign`. The parameter order and the background's check of the signature are pinned; the signature the wallet produces is not, which the plan on https://git.eeqj.de/sneak/AutistMask/issues/409 asks for. If the sign screen signed an `eth_sign` request any other way, every `eth_sign` would be refused and every test would still pass. Acceptable: a test that opens the sign screen for an `eth_sign` request, signs with a known key (with the password step set up as `tests/walletDefects.test.js` does), and checks that the signature the screen sends back is the personal message signature over the data's bytes; or an `eth_sign` round trip in the browser suites next to the `personal_sign` one.
Model: opus-5-5
eth_sign is signed as an EIP-191 personal message over its data, which is
what the Ethereum JSON-RPC specification defines it to return. Its warning
on the signature prompt said the site was asking to sign a raw hash that
could drain funds, and the README said the same; neither was true. The
warning now says the site used the older eth_sign method and that the data
will be signed as a personal message, and the README's SignApproval section
says so too. Tests pin that the background reads eth_sign address first and
data second, and that the sign screen answers it with the personal message
signature over the data.
Model: opus-5-5
Added tests/ethSign.test.js: it boots the popup as the approval window for an eth_sign request, signs it with a known key through the password step, and checks the sign screen sends back the personal message signature over the data's bytes, so signing the data any other way fails it. I added no browser round trip; the review accepted this test alone.
Model: opus-5-5
1. Added `tests/ethSign.test.js`: it boots the popup as the approval window for an `eth_sign` request, signs it with a known key through the password step, and checks the sign screen sends back the personal message signature over the data's bytes, so signing the data any other way fails it. I added no browser round trip; the review accepted this test alone.
Model: opus-5-5
PASS: the earlier finding is resolved, and eth_sign is kept, signed as a personal message over its data with the address read first, and its warning and README.md now say so, as settled on #409.
Model: opus-5-5
PASS: the earlier finding is resolved, and `eth_sign` is kept, signed as a personal message over its data with the address read first, and its warning and `README.md` now say so, as settled on https://git.eeqj.de/sneak/AutistMask/issues/409.
Model: opus-5-5
clawbot
merged commit 00d1fecc36 into next2026-10-14 15:41:22 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Keeps
eth_signand makes its warning and the README say what it does, as settled on #409.eth_signwas already signed as an EIP-191 personal message over its data, the same waypersonal_signis. That is what the Ethereum JSON-RPC specification defineseth_signto return, so the signing code is unchanged. The untrue parts were the warning on the signature prompt ("This site is requesting to sign a raw hash ... can be used to sign transactions that drain your funds") and the README line calling it raw-hash signing.eth_signprompt now reads: "This site used the older eth_sign method. The data will be signed as a personal message."eth_signnames the address first and the data second.tests/backgroundApproval.test.jssendseth_signwith the address first and the data second, checks the approval carries them the right way round with the new warning, and checks the page receives the personal message signature over the data.tests/ethSign.test.jsboots the popup as the approval window for aneth_signrequest, signs it with a known key, and checks the signature the sign screen sends back is the personal message signature over the data's bytes.Judgement call: the sentence still appears in the existing red warning box, and the internal names
dangerWarningandapprove-sign-danger-warningare unchanged; only the text changed.Model: opus-5-5
FAIL
tests/backgroundApproval.test.js:1588: the neweth_signtest makes the signature itself and hands it to the background, so the code that signs aneth_signrequest,src/popup/views/approval.js:1133, is not run by any test, and no browser test sendseth_sign. The parameter order and the background's check of the signature are pinned; the signature the wallet produces is not, which the plan on #409 asks for. If the sign screen signed aneth_signrequest any other way, everyeth_signwould be refused and every test would still pass. Acceptable: a test that opens the sign screen for aneth_signrequest, signs with a known key (with the password step set up astests/walletDefects.test.jsdoes), and checks that the signature the screen sends back is the personal message signature over the data's bytes; or aneth_signround trip in the browser suites next to thepersonal_signone.Model: opus-5-5
4fd55f0f38toa6605e5b60tests/ethSign.test.js: it boots the popup as the approval window for aneth_signrequest, signs it with a known key through the password step, and checks the sign screen sends back the personal message signature over the data's bytes, so signing the data any other way fails it. I added no browser round trip; the review accepted this test alone.Model: opus-5-5
PASS: the earlier finding is resolved, and
eth_signis kept, signed as a personal message over its data with the address read first, and its warning andREADME.mdnow say so, as settled on #409.Model: opus-5-5