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

Replace web3->ethers and JS->TS in RPC class #589

Open
wants to merge 1 commit into
base: confluence
Choose a base branch
from

Conversation

kautukkundan
Copy link
Contributor

What does this pull request do? Explain your changes. (required)

Specific updates (required)

  • convert rpc.js to rpc.ts and introduce types
  • replace web3.js methods with ethers.js methods
  • update occurences where RPC class is used to use ethers provider

How did you test each of these updates (required)

Does this pull request close any open issues?

closes #573

Checklist:

  • README and other documentation updated
  • All tests using yarn test pass

@RiccardoBiosas RiccardoBiosas self-requested a review June 1, 2022 14:53
Comment on lines +14 to +15
// Change block time using TestRPC call evm_setTimestamp
// https://github.com/numerai/contract/blob/master/test/numeraire.js
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these comments are now outdated and can be removed

export default class RPC {
constructor(public provider: ethers.providers.JsonRpcProvider) {}

sendAsync(method: string, arg: any[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we explicitly mark it as 'async' and also specify a return type? (I guess by using a generic since it is used in functions that return Promise<void> or Promise<someType>

await this.waitUntilBlock(targetBlock, seconds)
}

async getBlockNumberAsync() {
Copy link
Contributor

Choose a reason for hiding this comment

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

add return type

return this.sendAsync("evm_increaseTime", [time])
}

mine() {
Copy link
Contributor

Choose a reason for hiding this comment

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

mark it as async and since it seems in the codebase we are not expecting any return type i guess you can just use Promise<void>

return this.sendAsync("evm_mine", [])
}

async snapshot() {
Copy link
Contributor

Choose a reason for hiding this comment

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

specify return type

)
}

nextBlockMultiple(currentBlockNum: number, blockMultiple: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

specify return type

}
}

async waitUntilNextBlockMultiple(
Copy link
Contributor

Choose a reason for hiding this comment

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

specify return type

return this.sendAsync("evm_revert", [snapshotId])
}

async wait(blocks = 1, seconds = 20) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

return id
}

revert(snapshotId: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as the comment for the 'mine' method

@RiccardoBiosas
Copy link
Contributor

Good job starting the refactoring to typescript. I guess we can also improve the type annotations a bit

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.

Refactor RPC class
2 participants