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.

Sign up for GitHub

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

Jump to bottom

Fabo/voyager web #1804

Merged
merged 117 commits into from
Jan 16, 2019
Merged

Fabo/voyager web #1804

merged 117 commits into from
Jan 16, 2019

Conversation

faboweb
Copy link
Collaborator

@faboweb faboweb commented Jan 5, 2019 •
edited
Loading

Fabo, breaker of chains

https://wonderful-shaw-507bc9.netlify.com

Description:

Missing:

  • code cleanup
  • tests for wallet and keystore
  • fix unit tests
  • remove electron references and files
  •  repair tooling around nodes and account creation
  • add tx generation and signing
  • add ability to not generate new genesis every start
  • remove caddy binary
  • add test coverage
  • (later) repair reconnect
  • (later) new build process and documentation
  • (later) new e2e test setup (for now disable)
  • (later) refine google analytics import and usage
    • globals are now avoided

❤️ Thank you!

  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer

@jbibla
Copy link
Collaborator

jbibla commented Jan 6, 2019

holy smokes @faboweb this is so great.

sabau
sabau reviewed Jan 7, 2019
View reviewed changes
app/src/helpers/keystore.js Outdated Show resolved Hide resolved
sabau
sabau reviewed Jan 7, 2019
View reviewed changes
app/src/helpers/wallet.js Outdated Show resolved Hide resolved
sabau
sabau reviewed Jan 7, 2019
View reviewed changes
app/src/renderer/main.js Outdated Show resolved Hide resolved
@fedekunze
Copy link
Contributor

The balance header shows 0 because the testnet denom is stake and the account has STAKE

@faboweb faboweb changed the title [POC/DNM] Fabo/voyager web Fabo/voyager web Jan 14, 2019
@faboweb faboweb mentioned this pull request Jan 15, 2019
Closed
2 tasks
@sabau sabau mentioned this pull request Jan 15, 2019
Merged
2 tasks
fedekunze
fedekunze reviewed Jan 15, 2019
View reviewed changes
app/src/renderer/components/common/AnchorCopy.vue
@@ -31,6 +30,10 @@ export default {
required: true
}
},
data: () => ({
/* istanbul ignore next */
Copy link
Contributor

Choose a reason for hiding this comment

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

curious about what's istanbul

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

coverage reporter, this ignores the following line

app/src/renderer/connectors/lcdClient.js
}
}

let fetchAccount = argReq(`GET`, `/auth/accounts`, ``, true)

const keys = {
Copy link
Contributor

Choose a reason for hiding this comment

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

eventually we will need to call POST /keys/{name}/recover for key recovery right ?

app/src/renderer/connectors/lcdClient.js Show resolved Hide resolved
app/src/renderer/google-analytics.js Show resolved Hide resolved
app/src/renderer/main.js Show resolved Hide resolved
app/src/renderer/vuex/modules/send.js
async sendTx({ state, dispatch, commit, rootState }, args) {
if (!rootState.connection.connected) {
throw Error(
`Currently not connected to a secure node. Please try again when Voyager has secured a connection.`
Copy link
Contributor

Choose a reason for hiding this comment

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

how does the user know when Voyager secured the connection ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The connection indicator shows if the user is connected or not.
This error should actually never be called as all actions should be disabled when not connected.

app/src/renderer/vuex/modules/send.js
account_number: rootState.wallet.accountNumber, // TODO move into LCD?
chain_id: rootState.connection.lastHeader.chain_id,
gas: `50000000`,
generate_only: true
Copy link
Contributor

Choose a reason for hiding this comment

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

see my comment above, don't we need to add POST tx/sign too ?

app/src/renderer/vuex/modules/send.js

let requestMetaData = {
sequence: state.nonce,
name: `anonymous`,
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't we use the local key here (when not using signer app/ledger)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Stargate and therefor the keystore is remote in our case. We don't want to leave our keys in a remote location. Therefor we need to remove all the keys endpoints and can't sign using the SDK.

package.json
@@ -18,16 +18,14 @@
"build:testnets": "cd tasks/build/testnets && ./localBuild.sh",
"build:gaia": "cd tasks/build/Gaia && ./localBuild.sh",
"build:local": "node tasks/build/local/build",
"start": "yarn frontend & yarn backend skip-rebuild",
"start:new": "yarn frontend & yarn backend",
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add this to the README

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated the readme

tasks/gaia.js Outdated Show resolved Hide resolved
fedekunze and others added 2 commits January 16, 2019 10:16
@fedekunze @faboweb
Switch to photinos
760ccb3 
Co-Authored-By: faboweb <[email protected]>
@faboweb @fedekunze
Fabo/1821 improve testnet and output (#1829)
4cd9346 
* fix outputs and add verbose mode

* fixed third node issue

* Update tasks/build/local/helper.js

Co-Authored-By: faboweb <[email protected]>

* Update tasks/testnet.js

Co-Authored-By: faboweb <[email protected]>

* removed display of secondary accounts
@faboweb faboweb changed the base branch from develop to web January 16, 2019 12:01
sabau
sabau approved these changes Jan 16, 2019
View reviewed changes
Copy link
Contributor

@sabau sabau left a comment

Choose a reason for hiding this comment

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

Let's move to web (branch for now) 🎉

@sabau sabau merged commit 0e11960 into web Jan 16, 2019
@faboweb faboweb deleted the fabo/voyager-web branch January 23, 2019 21:22
This was referenced Feb 14, 2019
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fedekunze fedekunze fedekunze left review comments

@jbibla jbibla jbibla left review comments

@sabau sabau sabau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@faboweb @jbibla @fedekunze @sabau