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

Fix/testnet support #874

Merged
merged 17 commits into from
Aug 17, 2018
Merged

Fix/testnet support #874

merged 17 commits into from
Aug 17, 2018

Conversation

plondon
Copy link
Contributor

@plondon plondon commented Aug 15, 2018

TODO

  • Fix kvstore entries
  • Fix send
  • Fix add wallet account
  • Disable other currencies?

Description

Pull in network from wallet-options

Change Type

  • Feature
    Added testnet support

  • Bug Fix
    ~~1. The var network is assigned with settings.NETWORK_BITCOIN but we are using NETWORK as the name in config/index.js so its value is always undefined. Renamed NETWORK as NETWORK_BITCOIN

  1. Passing network as a param in actions.auth.register() function~~

Testing Steps

  1. Change wallet-options-v4 in config/ to use testnet instead of bitcoin network.
  2. Run yarn start:testnet
  3. Should successfully create a wallet

Code Checklist

  • Code compiles successfully (verified via yarn start)
  • No lint issues exist (verified via yarn lint)
  • New and existing unit tests pass (verified via yarn test)
  • README.md and other documentation is updated as needed

lzhuor and others added 12 commits August 5, 2018 22:49
- Fixed config/index.js by renaming 'NETWORK' to 'NETWORK_BITCOIN'
- Setting network config conditionally by checking environment
- Passing network from onSubmit function of Register component
- Fixed a few linting issues
- Rebased development branch
- Rebased development branch
- Add fallbackFees when mempool/fees fails
@plondon
Copy link
Contributor Author

plondon commented Aug 15, 2018

@lzhuor thanks again for your help, heres the PR

@lzhuor
Copy link
Contributor

lzhuor commented Aug 16, 2018

Thank you @plondon ! Greatly appreciate your help!

@@ -61,19 +61,19 @@ export const fromKeys = (entryECKey, encKeyBuffer, typeId) => {
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember if kvstore works with testnet addresses. Have you tried that @plondon ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually need to check this more because if I set a kvstore value (e.g for shapeshift state) then login breaks.

Note to self:

at fromObject (index.js:311)
    at from (index.js:137)
    at Function.push.../../node_modules/buffer/index.js.Buffer.from (index.js:149)
    at magicHash (index.js:37)
    at Object.verify (index.js:57)
    at KVStoreEntry.js:136
    at _curryN.js:34
    at _arity.js:22
    at KVStoreEntry.js:160
    at _curryN.js:34

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kvstore works now

@schnogz
Copy link
Contributor

schnogz commented Aug 16, 2018

Overall looks pretty good. Two feature requests though.

  1. Can we change the Webpack dev server wallet options to proxy to return wallet-options-v4.json with the network set to testnet. That way we dont have to manually modify the file.
  2. Similar to v3, we should have some sort of red warning banner in the header of the app that indicates testnet is in use. Hopefully people wont lose funds then.

@plondon plondon added this to the 4.3.x milestone Aug 16, 2018
@plondon plondon merged commit 066e547 into development Aug 17, 2018
@plondon plondon deleted the fix/testnet_support branch August 17, 2018 21:58
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.

None yet

5 participants