Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Feature/batpay-induction #75

Merged
merged 11 commits into from
May 16, 2019
Merged

Feature/batpay-induction #75

merged 11 commits into from
May 16, 2019

Conversation

azavalla
Copy link
Contributor

This change attempts to make BayPay's usage more easy-going for new users, by documenting BatpayJS library and making the full walkthrough work against a real node, until we introduce a better way of distributing the library and a simpler interface.

  • Adapted demo.js to run against a Geth client.
  • Documented relevant Batpay functions and demo.
  • Added eth-client client module, for Ethereum client version checking.
  • Added support for skipping blocks on clients other than ganache. Note that evm_mine is a RPC method only available on ganache-cli.
  • Some Batpay methods now RORO(Receive an object, return an object) for improved readability and maintenance.
  • Changed challengeBlocks and unlockBlocks testing configuration.
  • Improved naming, removed unused variables, general housekeeping.
  • Added Truffle's external script callback.
  • Renamed bat.js library to Batpay.js.
  • Updated README.

Changes were tested on a private network against a Geth client. Note that, it will probably won't work with Infura until we upgrade Truffle, or implement some kind of nonce-manager-middleware ( link, link and link ).

@azavalla azavalla requested a review from futo May 13, 2019 01:23
@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

Merging #75 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   98.47%   98.47%           
=======================================
  Files           6        6           
  Lines         394      394           
  Branches       32       32           
=======================================
  Hits          388      388           
  Misses          6        6

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dceac8...f8c955b. Read the comment docs.

@danielfx90
Copy link
Member

@azavalla looks great! A question, though.

truffle-hdwallet-provider has the shareNonce option that when set to false it creates a new WalletProvider that will track its own nonce-state. Given this, do you still think that we'll have the nonce issue?

We faced this same problem in Wibson v1. At that time, this parameter was not available. We used the workaround exposed in your first link. It would be by far better if the shareNonce option works as expected.

@azavalla
Copy link
Contributor Author

@danielfx90 I didn't know about that option! I'm running the demo with shareNonce: true to check whether that solves the nonce too low errors or not.

Was there a reason for setting the shareNonce option to false or can we just set it to true for all providers?

@danielfx90
Copy link
Member

@azavalla The default value is true. false means that it will use an internal provider to track the nonce locally. That's what we need. Did it work with false?

@azavalla
Copy link
Contributor Author

@danielfx90 The first thing I tried was with shareNonce: false (that's the value it was already set to), but it failed because of nonce too low errors.

As I understand it, when shareNonce is true, every instance of HDWalletProvider will have the same nonce-state, while when shareNonce is false, every new instance of HDWalletProvider will have it's own nonce-state. But in any case, the nonce is being tracked. From the truffle docs:

If false, a new WalletProvider will track its own nonce-state.

Now, when I set shareNonce: true I no longer get nonce too low error, but I randomly get the infamous Invalid JSON RPC response. This only happens to me when using Infura. It seems to be an ongoing issue.

I can run the demo but at some point it errors, different each time, so I haven't been able to reproduce the same error twice. The demo runs fine on a private network.

@danielfx90
Copy link
Member

@azavalla sounds great. Let's merge this then and use shareNonce: true

@danielfx90 danielfx90 merged commit 9d08de7 into master May 16, 2019
@danielfx90 danielfx90 deleted the feature/batpay-walkthrough branch May 16, 2019 18:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants