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

feat: Hardhat compatibility (Part 1 - Provider interface) #178

Merged

Conversation

pierobassa
Copy link
Member

@pierobassa pierobassa commented Oct 30, 2023

This PR adds a draft proposition for the Provider interface that aligns with ethers Provider which is used by hardhat.

A Provider is a class that allows to perform read-only access to the blockchain. (A initial draft for the Signer interface will be added later). A signer is a class that allows to perform write-access operations to the blockchain.

The following considerations have been made for the Vechain-SDK Provider:

  • estimateGas: TransactionRequest can be refactored to our TransactionBody (core/src/transaction/types.d.ts) and then internally we perform all operations needed (e.g., creation of Transaction object and calculation of estimateGas)

  • call: call can be implemented by using call() of account-visitor.ts or compat.ts This depends on driver-no-vendor that uses httpPost and creates a query for Simple net's query NetParam

  • resolveName: resolveName will not be used in first release. Maybe we can consider it in the future when VNS is supported.

  • sendTransaction will call sendTransaction method which will be implemented in driver. Internally we will get the raw tx needed to sendTransaction. TransactionRequest can be refactored to our Transaction (core/src/transaction/types.d.ts) in this case we need to create a Transaction object that is signed. This method will be used by the Signer classes

  • destroy: destroy needs to be implemented by the hardhat plugin or any other class that implements the Provider interface.

  • getBlockNumber: getBlockNumber will call getBlockNumber method which will be implemented in driver. to retrieve the latest block there is the blocks/best endpoint.

  • getNetwork: Get the connected Network (Mainnet, Testnet, Solo). We should return a Network object that contains at least the chainTag & network name. This must be looked into detail as we need to understand what hardhat requires exactly.

  • getFeeData: FeeData in ethers contains gasPrice, maxFeePerGas, maxPriorityFeePerGas. In vechain we need to understand if these are the same or if we need to return something else.

  • getBalance: getBalance will call getBalance method which will be implemented in driver-no-vendor.. This methods does not exist in connex. It will need to call the /accounts/{address} endpoint.. The endpoint also allows to specify the revision which is the block number to get the balance for.

  • getTransactionCount: This method is not needed as we do not have the incremental nonce logic as per Ethereum.

  • getCode: getCode will call getCode method which will be implemented in driver-no-vendor. This methods does not exist in connex. It will need to call the /accounts/{address}/code endpoint which returns the bytecode for the given address. You can also specify the revision which is the block number. We will have to change the getCode signature with appropriate parameters.

  • getStorage: getStorage will call getStorage method which will be implemented in driver-no-vendor. This methods does not exist in connex. It will need to call the /accounts/{address}/storage/{key} endpoint which returns the storage value for the given address and key. You can also specify the revision which is the block number. We will have to change the getStorage signature with appropriate parameters.

  • broadcastTransaction: broadcastTransaction is the same as sendTransaction as it does eth_sendRawTransaction in hardhat ethers plugin but returns a TransactionResponse. This time we are receiving a raw tx instead of the TransactionBody object. Which means we are broadcasting the tx already signed and encoded. can use sendTx of connex's driver

  • getBlock: getBlock will call getBlock method which will be implemented in driver-no-vendor. This methods does not exist in connex. It will need to call the /blocks/{revision} endpoint We will need to change the signature in order to specify blockId or blockNumber to get the block. Also, prefetchTxs is a boolean to specify whether to show transactions of the block or not. In thorest, the query param is 'expanded' which expands information about the transaction of the block. Note that transactions of the block are always present, if expanded is true then the transaction details are also present. Otherwise only the transaction hashes are present.

  • getTransaction: getTransaction will call getTransaction method which will be implemented in driver-no-vendor.
    This methods exists in driver. We can add the 'allowPending' parameter to specify whether to allow pending transactions or not. A pending transaction might have meta attribute null.

  • getTransactionReceipt: getTransactionReceipt will call getReceipt method which will be implemented in driver-no-vendor. This method will call the /transactions/{id}/receipt endpoint.

  • getTransactionResult: This method is not implemented by hardhat ethers plugin. And it's about tracing transactions and is not supported by vechain as far as I know.

  • lookupAddress: We do not use ENS (will be VNS) for now.

  • waitForTransaction: This method is not implemented by hardhat ethers plugin. We can make an improved implementation to the one in ethers' abstract-signer

  • waitForBlock: This method is not implemented by hardhat ethers plugin. Nore is it implemented by ethers' abstract-provider.

What's next?

Once we agree with the Provider interface, I'll start working on the provider class (e.g., abstract-provider) by opening atomic issues for each group of correlated methods

@pierobassa pierobassa linked an issue Oct 30, 2023 that may be closed by this pull request
6 tasks
@pierobassa pierobassa marked this pull request as draft October 30, 2023 20:51
@github-actions
Copy link

github-actions bot commented Oct 30, 2023

Test Coverage

Summary

Lines Statements Branches Functions
Coverage: 100%
100% (1328/1328) 100% (293/293) 99.64% (279/280)
Title Tests Skipped Failures Errors Time
core 350 0 💤 0 ❌ 0 🔥 1m 17s ⏱️
network 119 0 💤 0 ❌ 0 🔥 2m 0s ⏱️
errors 30 0 💤 0 ❌ 0 🔥 8.315s ⏱️

Copy link
Contributor

@rodolfopietro97 rodolfopietro97 left a comment

Choose a reason for hiding this comment

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

Left 1 comment. More can we remove these files form coverage count in order to not change temporary our 100% coverage :D

fabiorigam
fabiorigam previously approved these changes Oct 31, 2023
Copy link
Member

@fabiorigam fabiorigam left a comment

Choose a reason for hiding this comment

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

LGMT but I'm already working on the driver and driver-no-vendor stuff, so let's have a call before you start working on it

@pierobassa pierobassa changed the title feat: Hardhat compatibility (Part 1 - Provider) feat: Hardhat compatibility (Part 1 - Provider interface) Oct 31, 2023
Copy link

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: [email protected]

@fabiorigam fabiorigam dismissed their stale review December 12, 2023 12:53

A lot has changed

@rodolfopietro97 rodolfopietro97 linked an issue Dec 13, 2023 that may be closed by this pull request
12 tasks
@rodolfopietro97 rodolfopietro97 marked this pull request as ready for review December 18, 2023 14:01
Copy link
Member Author

@pierobassa pierobassa left a comment

Choose a reason for hiding this comment

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

LGTM! Left some minors.

Also gas module tests are failing locally

package.json Outdated Show resolved Hide resolved
packages/errors/src/model/generic/function.ts Show resolved Hide resolved
packages/ethers-adapter/Readme.md Outdated Show resolved Hide resolved
packages/ethers-adapter/Readme.md Outdated Show resolved Hide resolved
packages/ethers-adapter/package.json Outdated Show resolved Hide resolved
… github.com:vechainfoundation/thor-sdk-js into 132-alignment-with-hardhat-initial-public-functions
@rodolfopietro97 rodolfopietro97 merged commit 72c815a into main Dec 21, 2023
9 checks passed
@rodolfopietro97 rodolfopietro97 deleted the 132-alignment-with-hardhat-initial-public-functions branch December 21, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants