[go: nahoru, domu]

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

Store nonce in local file system and recover it when starting gateway #4505

Merged
merged 6 commits into from
Oct 22, 2021

Conversation

james-hummingbot
Copy link
Contributor
@james-hummingbot james-hummingbot commented Oct 19, 2021

┆Issue is synchronized with this Clickup task by Unito

@james-hummingbot
Copy link
Contributor Author
james-hummingbot commented Oct 19, 2021

@martinkou
I am using this as a reference:
https://github.com/level/level#usage

It mentions that you shouldn't need to run db.open(), but in yarn test:unit it complained that the database was not open. By adding db.open() before db.createReadStream() it reduced the number of errors.
https://github.com/level/level#dbopencallback

I have not seen any errors while running gateway by itself and making curl calls to it. Also running yarn jest test/chains/ethereum/evm.nonce.test.ts works fine. yarn test:unit does result in a number of errors and I am not sure why. It feels like something goes wrong when multiple threads are using db.

Here are some errors I see:

  • OpenError: IO error: lock gateway.level/LOCK: already held by process
  • Error [ERR_UNHANDLED_ERROR]: Unhandled error. (Error {})

@@ -56,6 +55,17 @@ export class Ethereum extends EthereumBase {
return Ethereum._instance;
}

async init(): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now init from nonceManager is async, so we need to override init from ethereum-base.


export class EVMNonceManager {
private static _instance: EVMNonceManager;
private _addressToNonce: Record<string, [number, Date]> = {};
private _delay: number | null = null;
private _initialized: boolean = false;
private _chainId: number = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't great, but making it null | number was also annoying, though it is probably more correct.

if (!this._provider || !this._delay) {
this._provider = provider;
this._delay = delay;
}

if (!this._initialized) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Load the nonces from the local storage file if it exists. This is async so it make init async.

private _ready: boolean = false;
private _initializing: boolean = false;
private _initPromise: Promise<void> = Promise.resolve();
protected _ready: boolean = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expose these as protected so the Ethereum class can override init

@james-hummingbot
Copy link
Contributor Author
james-hummingbot commented Oct 19, 2021

@martinkou
Here is a possible answer: Only a single process (possibly multi-threaded) can access a particular database at a time. That would explain why it works while running and in single tests, but not all the tests together. I wonder if this library is still a good solution?

https://github.com/google/leveldb#limitations

@james-hummingbot
Copy link
Contributor Author

@willyPat Thanks for finding the solution.

@james-hummingbot james-hummingbot marked this pull request as ready for review October 21, 2021 15:43
@martinkou martinkou merged commit 60a376e into feat/gateway-v2 Oct 22, 2021
@martinkou martinkou deleted the feat/gateway-v2_store-nonce branch October 22, 2021 18:28
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 this pull request may close these issues.

3 participants