Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

listAccounts() returns getTime(), sometimes #368

Closed
alexander-hagen opened this issue Jan 15, 2021 · 8 comments · Fixed by #373
Closed

listAccounts() returns getTime(), sometimes #368

alexander-hagen opened this issue Jan 15, 2021 · 8 comments · Fixed by #373
Assignees

Comments

@alexander-hagen
Copy link
Contributor

Usually, listAccounts() works fine and returns the balances of the requested account.


newbalances=await account._client.rest.account.listAccounts();
if(newbalances.length===undefined) { console.log("Newbalances:"+JSON.stringify(newbalances)); };

Sometimes it returns getTime(). Newbalances:{"iso":"2021-01-15T09:22:55.205Z","epoch":1610702575.205}

@alexander-hagen alexander-hagen changed the title listAccounts() returns listAccounts() returns getTime(), sometimes Jan 15, 2021
@alexander-hagen
Copy link
Contributor Author

Troubleshooting so far shows that this occurs at times when rate limiting is active.

@bennycode
Copy link
Owner

Hi @alexander-hagen, I guess that there is a call to getTime() every time you make a call to listAccounts(), am I right with that? If yes, then we can maybe cache the result of getTime() and only do it whenever this time skew gets outdated. This could save a lot of requests to Coinbase Pro.

@alexander-hagen
Copy link
Contributor Author

For every REST call, there also is a request to GET /time. I was thinking to get rid of skew altogether as CoinbasePro permits requests with up to 30 seconds time discrepancy. Alternatively, periodically updating is perhaps a better alternative.

@bennycode
Copy link
Owner

Let's do the caching approach: We fetch the skew once and when Coinbase Pro reports that the time is out of sync, we will update it.

@alexander-hagen
Copy link
Contributor Author

I now also have a case where it happens while there no rate limiting. It still may be related to the GET /time though. I will do some tests without the GET /time and let you know

@bennycode bennycode linked a pull request Jan 21, 2021 that will close this issue
@bennycode bennycode self-assigned this Jan 21, 2021
@bennycode
Copy link
Owner

Hi @alexander-hagen, I built caching for the time skew. Can you please try coinbase-pro-node@2.16.4?

@alexander-hagen
Copy link
Contributor Author

OK, let me have a try. I have been running with local time only for the past past week and didn't have any the issue since then:

    getTime() {
        return __awaiter(this, void 0, void 0, function* () {
          const now=new Date();
          const epoch=now / 1000;
          const iso=now.toISOString();
          const data={"iso": now,"epoch": epoch};
          return data;
        });
    }

@alexander-hagen
Copy link
Contributor Author

Looks good. Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants