urbit / bridge

An application for interacting with Azimuth.
MIT License
95 stars 25 forks source link

Cannot reset networking keys #1078

Open ykr973 opened 1 year ago

ykr973 commented 1 year ago

Describe the bug Cannot reset my networking keys.

To Reproduce 1) https://bridge.urbit.org 2) Login with Walletconnect 3) Click the > 4) Click OS 5) Click Reset next "Reset Network Keys" 6) Check "Factory Reset" 7) Click "Reset Network Keys" 8) Approve transaction in wallet

Error message shown shown below results almost every time, the two times it didn't appear 'tt undefined' was displayed in on the web browser in the same space where "Request rejected" appears if the transaction is rejected. There is no actual network problem. I tried multiple connections, with and without VPN. If I reject the transaction it immediately shows "Transaction rejected" in red indicating there is no actual network problem nor any connection problem between the dapp and the wallet.

Expected behavior Create a transaction and reset networking keys.

Screenshots

IMG

reject

Desktop (please complete the following information): macOS 13.0.1

Safari, Firefox, Brave, Chrome

Login method (please complete the following information): WalletConnect

Additional context N/A

tomholford commented 1 year ago

@ykr973 Thanks for reporting this issue. Which wallet are you using with WalletConnect?

tomholford commented 1 year ago

Cross-posting a submission from another issue, here's a screenshot:

image

ykr973 commented 1 year ago

@ykr973 Thanks for reporting this issue. Which wallet are you using with WalletConnect?

@tomholford Argent

Argent asked "if the dapp itself supports EIP-1271 signatures"?

Also, tried with a tablet, a windows computer, some other browsers - all behave the same.

Argent suggested that maybe the bridge blocks signatures from contract addresses. While it's perhaps not important to specifically support Argent, I think some people might want to use a multi-sig like Gnosis Safe and if Argent's guess is correct it sounds like that wouldn't work either.

vvisigoth commented 1 year ago

I wonder if this would be solved by updating WalletConnect support?

shrugs commented 1 year ago

WC update would not change this behavior afaik

am i right in assuming that this urbit id is on the rollup? if this ship is managed on mainnet, everything should be hunkydory because the app would submit a configureKeys transaction to argent directly, which it would be able to execute it on-chain as normal.

my understanding is that the issue here is that the signer is a smart contract and follows eip-1271 signature standard to "sign" messages, but that means that clients of those smart contracts (i.e. gnosis safe or argent apps) don't have anything valid to return to the caller, so they return "0x". this is probably the cause of the .length issue, though i can't find that exact codepath at the moment)

to verify that a smart contract does indeed want some thing to be 'signed', one must query the contract with the isValidSignature(...) method and ensure that it returns the magic value (specified in EIP-1271) — my assumption, without digging to much deeper, is that the urbit roller rpc doesn't support EIP-1271 signatures, and returns the request rejected error because the arguments fail validation.

the solution is to implement EIP-1271 validation either on the client side or on the roller api side. the client-side code is very simple and looks like an exponential backoff retry that queries the chain as above, waiting for that signature to be valid. if the roller api server has a queue of some kind to keep not-yet-valid messages in, that polling can be done in there as well. regardless, the roller api must also validate the signature on-chain (instead of recovering an EOA's signature as it does now) to verify that the request is legitimate. and then it should work