dsrvlabs / celo-extension-wallet Goto Github PK

Brandon | RADAR — Yesterday at 2:01 PM
I'm having a very difficult time debugging this, but Celo extension + Ledger produces intermittent internal RPC errors.  I've reproduced them between machines when doing simple zero-amount actions like claiming from the LP incentive pools, but then I build a seemingly-identical transaction manually and it succeeds.
the intermittent error is insufficient funds for gas * price + value + gatewayFee
this is not a nonce error, it's not a stateful expiration (claim can't expire or become invalid like a swap can), and when it appears it reproduces on multiple unrelated, unsynced machines with the same setup.  using the Ledger directly without the extension doesn't encounter the problem.
I noticed that transactions built by accessing the site with the Celo extension show no gateway fee, while transactions built manually show a gateway fee of 0 and a gateway address of 0x00..0000.  not sure if that's related, but it's a way that Celo transactions differ from the Ethereum RPC and maybe an opportunity for type confusion somewhere in the implementation
the account has tons of CELO, so that shouldn't be the issue either
I know that Celo transactions can opt into paying the fee in cUSD instead of CELO.  not sure how that's implemented or why it would be causing an issue here, but that's another possibility.
(this account holds no cUSD)
based on the search, a number of people have had issues with Extension + Ledger, but no clear lead on the cause

TQT — Yesterday at 3:25 PM
@Brandon | RADAR not sure if related but if you have one tab open with your ledger connected to it, it gives an RPC error on the others iirc

Brandon | RADAR — Yesterday at 3:30 PM
I get the same error when I submit the rawtx to the forno.celo.org RPC endpoint
I think there's something wrong with Forno's gas math
the rawtx RLP looks properly encoded.  12 elements, everything looks the way I expect it to
Ledger involvement might be a red herring

Brandon | RADAR — Yesterday at 4:15 PM
ah.  figured it out.  Celo Extension + Ledger is using the wrong value for v
this makes it tx-dependent, because the correct v value depends on the TX hash
and when the RPC says insufficient funds for gas * price + value + gatewayFee, this actually means bad signature
whelp, that blew up my entire day

yourbuddyconner — Yesterday at 4:18 PM
Nice debugging damn

Brandon | RADAR — Yesterday at 4:19 PM
based on this, Celo Extension + Ledger transactions should fail about half the time
so this is a failure in test coverage plus a failure to pick this error out of user-submitted reports

Brandon | RADAR — Yesterday at 4:21 PM
kept thinking about giving up, but I know the whole stack and it just wasn't adding up
"signer creates an invalid signature half the time" is not something you check for in the first twenty things :joy:

ChaimG — Yesterday at 4:37 PM
Nice update on the App showing user phone number when connected to Valora App :ube:

heavykevy — Yesterday at 4:41 PM
Brandon = :goat:

Brandon | RADAR — Yesterday at 4:43 PM
background: v is used to recover the public key that created a signature, like on a transaction.  most chains follow EIP-155 where v is specified as (2 x chainId) + 35 + {0,1}; whether 0 or 1 are added depends on the parity of the y-value of the curve point corresponding to r.  Celo Mainnet network ID is 42220, so valid values of v are 84475 (0x149b) or 84476 (ox149c).  For whatever reason, the Celo Extension is doing the math wrong on whether to add 0 or 1 to v.  signature verification could technically just iterate through the possible values of v to recover the pubkey, and providing v just speeds the process up by avoiding some iteration, but apparently the RPC signature verification assumes v is correct and is returning that gas error because it's not recovering the pubkey correctly.
so half of the transactions created by Celo Extension + Ledger are, randomly, invalid
all client-side software needs to be overhauled anyway with the RPC changes coming in Donut, so it's a great time to fix this