clients: Support Brave wallet default

[aefhm]

Description

Close #385 maybe.

Problem

Wagmi is concerned with bad implementations of EIP-1193 and wants to check that the removeListener function exists. However, the Brave wallet overrides ethereum properties and does not like the way we currently use Proxy objects, giving a typeerror upon a get call.

Our current implementation is a classic Proxy pattern in order to preserve this.

const proxy = new Proxy(aSecret, {
  get(target, prop, receiver) {
    const value = target[prop];
    if (value instanceof Function) {
      return function (...args) {
        return value.apply(this === receiver ? target : this, args);
      };
    }
    return value;
  },
});

simplified to:

sapphire-paratime/clients/js/src/provider.ts

Line 211 in 42c1a69

return typeof value === 'function' ? value.bind(upstream) : value;

We can repro error via:

class Secret {
  #x = 1;
  x() {
    return this.#x;
  }
}

const aSecret = new Secret();
Object.defineProperties(aSecret, {
  removeListener: {
    value: function () {},
    writable: false,
  }
});
const proxy = new Proxy(aSecret, {
  get(target, prop, receiver) {
    const value = target[prop];
    if (value instanceof Function) {
      return function (...args) {
        return value.apply(this === receiver ? target : this, args);
      };
    }
    return value;
  },
});
console.log(proxy.removeListener);

One solution is to not forward context in order to maintain the function signature for Brave browsers as noted in this PR.

[netlify]

✅ Deploy Preview for oasisprotocol-sapphire-paratime canceled.

Name	Link
🔨 Latest commit	3cc95f8
🔍 Latest deploy log	https://app.netlify.com/sites/oasisprotocol-sapphire-paratime/deploys/6722b98ef398b800083f2200

[aefhm]

One other solution mentioned would involve giving up the Proxy which currently prevents developers from accidentally double wrapping the provider. We ostensibly need that differentiation if but only for debugging purposes. Otherwise, I think creating a singleton factory would not necessarily be a clean solution, requiring a dev to continually reference the wrapped SapphireProvider.

[aefhm]

Checking if @lukaw3d knows of a better magical JavaScript solution. 🙏🧙

[CedarMist]

It should be possible to remove the proxy contract and use some kind of WeakRef singleton for providers instead?

Or... just don't do either.

As long as there's a test for using double-wrapped providers to make sure they don't mess up encryption & decryption.

[aefhm]

Brave makes the upstream a Proxy. 😆

[CedarMist]

Can we do something like instead?

return (...args) => Reflect.apply(value, upstream, args)

Which will avoid the .bind call

[aefhm]

Can we do something like instead?

return (...args) => Reflect.apply(value, upstream, args)

Which will avoid the .bind call

Uncaught (in promise) TypeError: 'get' on proxy: property 'removeListener' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected 'function(e,t){var n,r,i,o,s;if(u(t),void 0===(r=this._events))return this;if(void 0===(n=r[e]))return this;if(n...<omitted>...s}' but got '(...args) => Reflect.apply(value, upstream, args)')
...

😭

[aefhm]

Can we do something like instead?

return (...args) => Reflect.apply(value, upstream, args)

Which will avoid the .bind call

Uncaught (in promise) TypeError: 'get' on proxy: property 'removeListener' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected 'function(e,t){var n,r,i,o,s;if(u(t),void 0===(r=this._events))return this;if(void 0===(n=r[e]))return this;if(n...<omitted>...s}' but got '(...args) => Reflect.apply(value, upstream, args)')
...

😭

Still generates typeerror.

Edit. I think the complication comes from the fact that upstream will sometimes be an Object, and sometimes a Proxy (Brave), so staying as a Proxy in our wrapper seems to be only path forward...

[aefhm]

I think maintaining the Proxy object is a reasonable for now in order to support Brave or any other upstream that hijacks the provider with their trap. What do you think @CedarMist?