From 781cdf39606dcad0dcdbd4ae71764ec0aca13aea Mon Sep 17 00:00:00 2001 From: Philip Monk Date: Tue, 27 Apr 2021 17:23:01 -0700 Subject: [PATCH] naive: alter signatures to match personal_sign --- pkg/arvo/app/naive.hoon | 4 ++- pkg/arvo/lib/azimuth.hoon | 5 ++- pkg/arvo/lib/naive.hoon | 65 ++++++++++++++++++++++++++++------- pkg/arvo/ted/eth-watcher.hoon | 4 ++- pkg/arvo/tests/lib/naive.hoon | 36 +++++++++++++++++-- 5 files changed, 96 insertions(+), 18 deletions(-) diff --git a/pkg/arvo/app/naive.hoon b/pkg/arvo/app/naive.hoon index 5f63459d22..1a5b9087ed 100644 --- a/pkg/arvo/app/naive.hoon +++ b/pkg/arvo/app/naive.hoon @@ -1,12 +1,14 @@ /- eth-watcher /+ ethereum, azimuth, naive, default-agent, verb, dbug /* snap %eth-logs /app/naive/logs/eth-logs +:: =/ last-snap :: maybe just use the last one? - %+ roll snap + %+ roll `(list event-log:rpc:ethereum)`snap |= [log=event-log:rpc:ethereum last=@ud] ?~ mined.log last (max block-number.u.mined.log last) +:: =, jael |% ++ app-state diff --git a/pkg/arvo/lib/azimuth.hoon b/pkg/arvo/lib/azimuth.hoon index 80cbb0462a..6cb55598c7 100644 --- a/pkg/arvo/lib/azimuth.hoon +++ b/pkg/arvo/lib/azimuth.hoon @@ -78,7 +78,7 @@ :: # constants :: :: contract addresses - ++ contracts mainnet-contracts + ++ contracts ropsten-contracts ++ mainnet-contracts |% :: azimuth: data contract @@ -129,6 +129,9 @@ ++ delegated-sending 0x3e8c.a510.354b.c2fd.bbd6.1502.52d9.3105.c9c2.7bbe :: + ++ naive + 0xb581.01cd.3bbb.cc6f.a40b.cdb0.4bb7.1623.b5c7.d39b + :: ++ launch 4.601.630 ++ public launch -- diff --git a/pkg/arvo/lib/naive.hoon b/pkg/arvo/lib/naive.hoon index 512638d088..564248a7e7 100644 --- a/pkg/arvo/lib/naive.hoon +++ b/pkg/arvo/lib/naive.hoon @@ -31,15 +31,30 @@ :: :: TODO: could remove `ship` from most txs since it's in `from` :: -:: TODO: hmm i don't think wraps can be done easily? because how do -:: you keep track of intra-wrap ownership changes. you need to verify -:: the eth signature outside, then verify owner as you go along -:: :: TODO: secp needs to not crash the process when you give it a bad :: v/recid. See #4797 :: +:: TODO: probably should make opcode 0 a no-op and generally ensure +:: that 0 is not a valid tx, or that it's a no-op that we don't even +:: send to verify (not sure the verify code can handle a 0 tx) +:: +:: TODO: add chainId, maybe just everything from +:: signTypedData_v4/EIP-712. If we use that, we need to determine +:: whether EIP-712 is supported by the relevent wallets. Looks like +:: ledger does, but maybe not Trezor? Is there a way to hack around +:: it? +:: +:: Okay, my understanding is we can use personal_sign for metamask, +:: trezor, and ledger, which means prepending each piece of signed data +:: with '\19Ethereum Signed Message:\0a' and then the length of the +:: signed data. We should also include chain_id and maybe other stuff +:: from the domain separator in EIP-712. But need to follow up on: +:: https://github.com/ethereum/go-ethereum/issues/14794 +:: +:: In any case, a signature version number sounds like a good idea +:: /+ std -=> => std +=> =+ std :: Laconic bit :: =| lac=? @@ -256,7 +271,6 @@ =^ sig batch (take 3 65) =. len.batch 0 =/ orig-batch rest.batch - :: Single tx =/ res=(unit [=tx batch=_batch]) parse-tx ?~ res ~ @@ -352,7 +366,15 @@ %vote voting-proxy.own.u.point %transfer transfer-proxy.own.u.point == - =/ signed-data (dad [5 1] nonce.need raw.raw-tx) + :: TODO: do we need to preserve the length of the raw tx? + :: + =/ prepared-data (dad [5 1] nonce.need raw.raw-tx) + =/ signed-data + %- keccak-256:keccak:crypto + %- as-octs:mimes:html + %^ cat 3 '\19Ethereum Signed Message:\0a' + %^ cat 3 (ud-to-len (met 3 prepared-data)) + prepared-data =/ dress (verify-sig sig.raw-tx signed-data) ?~ dress | @@ -363,15 +385,33 @@ |= [sig=@ txdata=@] ^- (unit address) |^ + :: Reversed of the usual r-s-v order because Ethereum integers are + :: big-endian + :: =^ v sig (take 3) - =^ r sig (take 3 32) =^ s sig (take 3 32) + =^ r sig (take 3 32) + :: In Ethereum, v is generally 27 + recid, and verifier expects a + :: recid. Old versions of geth used 0 + recid, so most software + :: now supports either format. See: + :: + :: https://github.com/ethereum/go-ethereum/issues/2053 + :: + =? v (gte v 27) (sub v 27) (verifier txdata v r s) :: ++ take |= =bite [(end bite sig) (rsh bite sig)] -- + :: ASCII-encode length + :: + ++ ud-to-len + |= n=@ud + ^- @t + ?~ n + *@t + (cat 3 $(n (div n 10)) (add '0' (mod n 10))) -- :: ++ ship-rank @@ -688,10 +728,10 @@ [%point ship %voting-proxy *address] [%point ship %transfer-proxy *address] == - =: address.spawn-proxy.own.point *address - address.management-proxy.own.point *address - address.voting-proxy.own.point *address - address.transfer-proxy.own.point *address + =: address.spawn-proxy.own.point *address + address.management-proxy.own.point *address + address.voting-proxy.own.point *address + address.transfer-proxy.own.point *address == `[:(welp effects-1 effects-2 effects-3) point] :: @@ -869,7 +909,6 @@ :: Received log from L1 transaction :: (receive-log state event-log.input) -%+ debug %batch :: Received L2 batch :: (receive-batch verifier state batch.input) diff --git a/pkg/arvo/ted/eth-watcher.hoon b/pkg/arvo/ted/eth-watcher.hoon index 60d0d5ee90..5e2303548a 100644 --- a/pkg/arvo/ted/eth-watcher.hoon +++ b/pkg/arvo/ted/eth-watcher.hoon @@ -88,7 +88,7 @@ ?: (lth latest-number (add number.pup zoom-margin)) (pure:m pup) =/ up-to-number=number:block - (min (add 1.000.000 number.pup) (sub latest-number zoom-margin)) + (min (add 10.000.000 number.pup) (sub latest-number zoom-margin)) |- ~& > [%zooming number.pup up-to-number] =* loop $ @@ -101,6 +101,8 @@ :: there are a lot events belonging to all the pre-ethereum ships :: being established on-chain. By reducing the step, we avoid crashing. :: + ?. =(contracts:azimuth mainnet-contracts:azimuth) + zoom-step ?: ?| &((gte number.pup 6.951.132) (lth number.pup 6.954.242)) &((gte number.pup 7.011.857) (lth number.pup 7.021.881)) == diff --git a/pkg/arvo/tests/lib/naive.hoon b/pkg/arvo/tests/lib/naive.hoon index 611ceb1c06..830da03ea6 100644 --- a/pkg/arvo/tests/lib/naive.hoon +++ b/pkg/arvo/tests/lib/naive.hoon @@ -55,8 +55,15 @@ :: ++ sign-tx |= [pk=@ nonce=@ud tx=@] ^- @ - =+ (ecdsa-raw-sign:secp256k1:secp:crypto (dad:naive 5 nonce tx) pk) - (cat 3 (can 3 1^v 32^r 32^s ~) tx) + =/ prepared-data (dad:naive 5 nonce tx) + =/ sign-data + %- keccak-256:keccak:crypto + %- as-octs:mimes:html + %^ cat 3 '\19Ethereum Signed Message:\0a' + %^ cat 3 (rsh [3 2] (scot %ui (met 3 prepared-data))) + prepared-data + =+ (ecdsa-raw-sign:secp256k1:secp:crypto sign-data pk) + (cat 3 (can 3 1^v 32^s 32^r ~) tx) :: ++ l1 |% @@ -430,4 +437,29 @@ =^ f state (n state %bat (transfer-point:l2 0 ~palsep-picdun (key ~palsep-picdun) %transfer &)) owner.own:(~(got by points.state) ~palsep-picdun) :: +++ test-metamask-signature ^- tang + =/ meta-owner=address + (hex-to-num:ethereum '0xb026b0AA6e686F2386051b31A03E5fB95513e1c0') + =/ tx 0x123.0000.0102.0a00.0001.0200 + =/ sig + %- hex-to-num:ethereum + :: %^ cat 3 '0xbcee11aad81466d8693571bdd020a2cc8ca7cd4a717bbfdedbe5d5296b596005' + :: '211e6c1a804ea0489ac15ff1dca7a0803f61c2fb473701d100dc9c07bbe6ba6f1c' + :: '0xdede6cb45463d5822e2558cd0aec6835c6500acf928754f7147bc066eaa1f5bb5913d66292e0f5c368611dc8fe2a9635b4d692ee64684a73bb581f31ec6bbefa1c' + :: Must reverse endianness of tx to sign in metamask + %^ cat 3 + '0x5b85936ab7b9db8d72416648e6eb1b844a4545ddb7c7c646a74bc3a4fb001a2' + '8583bf12ca837b289036a6cc9e6359ed07dda2b87929b5dd7189a3057a395341f1c' + :: + %+ expect-eq + !> [0x123 0] + :: + !> + =| =^state:naive + =^ f state (init-marbud state) + :: =^ f state (n state %bat (transfer-point:l2 0 ~marbud (key ~marbud) %own &)) + :: =^ f state (n state %bat (set-transfer-proxy:l2 1 ~marbud %own 0x123)) + =^ f state (n state %bat (transfer-point:l2 0 ~marbud meta-owner %own &)) + =^ f state (n state %bat (cat 3 sig tx)) + transfer-proxy.own:(~(got by points.state) ~marbud) --