GoodDollar / GoodDAPP

GoodDollar.org Wallet is the simplest access point to Claim your daily G$. It Is based on web3 and React native web.
good-dapp.vercel.app
MIT License
101 stars 52 forks source link

4227 rpc issues #4228

Closed L03TJ3 closed 4 months ago

L03TJ3 commented 4 months ago

Description

We get reports from users where claiming fails. After QA it seems to be related to rpc-issues (after just switching chains). Besides these reports, we already have a lot of rpcs errors on sentry.

Done fixes:

Wip fixes so far:

About # (link your issue here) https://github.com/GoodDollar/GoodDAPP/issues/4227 https://github.com/GoodDollar/GoodDAPP/issues/4226

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

vercel[bot] commented 4 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
goodwallet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 28, 2024 1:46pm
2 Ignored Deployments | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **gooddollar-delta** | ⬜️ Ignored ([Inspect](https://vercel.com/gooddollarteam/gooddollar-delta/3e8wqVATrzzwQ1PTB5Ej8Gj3W5L6)) | [Visit Preview](https://gooddollar-delta-git-4227-rpc-issues-gooddollarteam.vercel.app) | | Feb 28, 2024 1:46pm | | **goodid** | ⬜️ Ignored ([Inspect](https://vercel.com/gooddollarteam/goodid/6g793jJXshTQrXxM94K8sho3FPy4)) | [Visit Preview](https://goodid-git-4227-rpc-issues-gooddollarteam.vercel.app) | | Feb 28, 2024 1:46pm |
L03TJ3 commented 4 months ago

@johnsmith-gooddollar @sirpy Open questions:

  1. Currently when you switch chains, loggedProviders is cleared. Is this a wanted behavior or should we cache this somehow?

  2. if 1 is yes, maybe we should clear it periodically? for example for onfinality on a per minute basis?

  3. we are not the only ones using the MultipleHTTPPRovider. Web3Provider is passed down to WalletChat widget which also uses the httpProvider. We don't necessarily control their calls, not sure yet if this is relevant or if they do any extensive calls (not debugged far enough yet). just pointing it out to consider

johnsmith-gooddollar commented 4 months ago
  1. it shouldn't be cleared. make it static class member:

export class MultipleHttpProvider extends HttpProvider {
  static loggedProviders = new Map()

  constructor(endpoints, config) {
    const [{ provider, options }] = endpoints // init with first endpoint config
    const { strategy = 'random', retries = 1 } = config || {} // or 'random'

    log.debug('Setting default endpoint', { provider, config })
    super(provider, options)

    log.debug('Initialized', { endpoints, strategy })

    assign(this, {
      endpoints,
      strategy,
      retries,
    })
  }

  send(payload, callback) {
    const { endpoints, strategy, retries } = this
    const { loggedProviders } = MultipleHttpProvider

    // shuffle peers if random strategy chosen
  1. no, we shouldn't clear periodically. each RPC url should be logged just once per user session/refresh in the case of connection error

  2. as we decided do not implement fallback there, I think we could ignore and do not log also errors. As an option, if this provider supports events we could listen for errors and log them using the same rule (any connection issue logged once per session per each rpm url). I think we need also ask @sirpy