why the jsonRpc request id is a const value 42

[BigMurry]

I'm using ethers.js to connect my dapp with Trust Wallet.

But I found that, when I call signer.sendMessage, Trust Wallet pop up the sign message dialog, then I click 'OK'. But the signature never returns.

const provider = new ethers.providers.Web3Provider(window.web3.currentProvider);
try {
  const signer = provider.getSigner();
  const sig = await signer.signMessage('my messages');
  console.log(sig); // this line never hit
} catch (e) {
  console.log(e); // this line never hit
}

// also tried this way, the same result.
const sig = await provider.send('personal_sign', [ethers.utils.hexlify(ethers.utils.toUtf8Bytes('some message')), myaddr.toLowerCase()]);

When I dig into this, I found that Trust Wallet use the payload.id in sendAsync as the callbacks map id.
https://github.com/TrustWallet/trust-web3-provider/blob/a2b6adf070a8a4040ae325f0022db6480a893218/src/index.js#L89-L101

So, my question is why the request.id is a constant value 42?

ethers.js/src.ts/providers/web3-provider.ts

Lines 67 to 74 in 04c92bb

	return new Promise((resolve, reject) => {
	var request = {
	method: method,
	params: params,
	id: 42,
	jsonrpc: "2.0"
	};

[ricmoo]

The reason it is hard-codes is as a flare to me. The ID should not matter, since there is no multiplexing (since each call is in a new connection), and when I see a 42 (a number I personally recognize when I see it in server logs, etc) I realize it was actually a call ethers performed.

Are you saying that it works the first time, but not the second and that if you modify this to increment it works?

This is actually a potential attack vector in Trust Wallet then, and we should open a ticket to have it fixed immediately.

It is fairly easy to make a unique ID each time, it has just been useful to me in the past to see when it is my request vs. another library or hand-crafted request.

[BigMurry]

Yes, I've confirmed that the constant id makes Trust wallet could not send the rpc response back.

I create a forked version of ethers.js and comment the id: 42 line out, then Trust wallet can send the response back without any problems.

Trust wallet (more specific: trust-web3-provider) uses the request id to identify each request, and sends the response back by using the id to find the callback function.

The pseudo code:

class TrustWeb3Provider {
  constructor() {
    this.callbacks = new Map;
  }

  sendAsync(payload, callback) {
    this._sendAsync(payload).then(data => callback(null, data));
  }

  _sendAsync(payload) {
    return new Promise((resolve ,reject) => {
      // if the payload does not have a id of type 'number', it will create a random new id
      // if the payload already has one id, it will use the id as the callback map's key
      this.callbacks.set(payload.id, (err, data) => {
        if (err) {reject(err);} else {resolve(data);}
      });

       // make postMessage to native code to do the work
       // and when the postMessage handler is done with the work, it will call the window.ethereum.sendResonse(id, result)
    });
  }

  sendResponse(id, result) {
    const callback = this.callbacks.get(id); // so the same id in ethers.js will cause problems here
    callback(null, result);
  }
}

[mrwillis]

Wow. I can actually confirm we had a very similar issue with trust wallet. We couldn't figure out what was going on and it finally makes sense.

[cellog]

The id should not be hard-coded. https://ethereum.stackexchange.com/questions/50647/json-rpc-api-id

If you ever implement a websocket provider, it will break. Trust wallet aside, this is a bad practice. For example, when writing unit tests using nock to simulate a json-rpc server, as in https://github.com/unlock-protocol/unlock/blob/master/unlock-js/src/__tests__/helpers/nockHelper.js, if tests are set up incorrectly, it's possible to have a test pass with the nocks from a prior test. When id increases with each request, this would instead trigger a mismatch.

This is also a subtle difference with web3 that makes migration slightly more complex.

It is fairly easy to make a unique ID each time, it has just been useful to me in the past to see when it is my request vs. another library or hand-crafted request.

This convenience is outweighed by the concerns above, yes? If you really need this feature, why not make a configuration option that allows setting the id to a constant value, but default to increasing id?

[ricmoo]

The WebSocket version will certainly not hard-code this, which is why I've been holding off on this change, because the web socket version will have an abstracted nonce, and the abstraction will make both implementations simpler. :)

If you read that specification though, it does not specify the client should not hard-code (or even re-use) an ID; all that is important is that the server responds with the same ID in the response. I've read that part over many times before deciding it was okay to hard-code it, back in the day. A little bit of effort will be necessary to minimize privacy leakage when switching to using a separate nonce though. Otherwise one application may be able to see the rate of requests from other applications, and may be able to use timing information to do other weird timing-side-channel attacks... Obviously not a huge concern, but something I'll put at least a little effort into. :)

All I wanted to point out is that if this is causing problems with a client, that indicates that the client has a fairly serious security vulnerability. Imagine if when viewing google, the image you saw on their landing page was one from my site, because they both use the same filename, "logo.gif". Or worse, if the script loaded was from my site, because we both called it "script.js". Multiplexing keys should not be shared across connections, and in this case it sounds like across domains.

Depending on the level of this pool being shared, imagine two different each requesting an ENS name; there is no guarantee that unrelated JSON-RPC hosts won't collide IDs, in which case each App may get the other's ENS name, sending the wrong assets to the wrong use.

Anyways, it will be changed, but a lot of blockchain events have come up lately (ETHCapeTown was awesome, and Ethereal and ETHNewYork is next week), and other random things that I have had to take care of.

[ricmoo]

This should be fixed in 4.0.28. Please let me know if you still have any issues.

Thanks! :)