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

Colw/transaction service #3180

Merged
merged 52 commits into from
Dec 6, 2019
Merged

Colw/transaction service #3180

merged 52 commits into from
Dec 6, 2019

Conversation

faboweb
Copy link
Collaborator

@faboweb faboweb commented Nov 22, 2019

Description:

Requires updated Lunie-API that fetches accountNum and sequence. Updated.

Set VUE_APP_ENABLE_TX_API to true to enable.

Thank you! 🚀


For contributor:

  • Added changes entries. Run yarn changelog for a guided process.
  • Reviewed Files changed in the github PR explorer
  • Attach screenshots of the UI components on the PR description (if applicable)
  • Scope of work approved for big PRs

For reviewer:

  • Manually tested the changes on the UI

@codecov
Copy link

codecov bot commented Nov 25, 2019

Codecov Report

Merging #3180 into develop will increase coverage by 1.75%.
The diff coverage is 90.9%.

@@            Coverage Diff             @@
##           develop   #3180      +/-   ##
==========================================
+ Coverage    88.95%   90.7%   +1.75%     
==========================================
  Files          126     126              
  Lines         1919    1958      +39     
  Branches       291     298       +7     
==========================================
+ Hits          1707    1776      +69     
+ Misses         205     177      -28     
+ Partials         7       5       -2
Impacted Files Coverage Δ
src/components/common/TmSessionHardware.vue 100% <ø> (ø) ⬆️
src/components/staking/Undelegations.vue 100% <ø> (+58.33%) ⬆️
src/components/staking/TableUndelegations.vue 28.57% <ø> (+6.34%) ⬆️
config.js 100% <100%> (ø) ⬆️
src/ActionModal/components/ActionModal.vue 95.09% <100%> (+8.46%) ⬆️
src/ActionModal/utils/ActionManager.js 100% <100%> (ø) ⬆️
src/ActionModal/utils/MessageConstructor.js 88.88% <75%> (+28.88%) ⬆️

@@ -46,3 +61,167 @@ export const getMultiMessage = async (context, messages) => {
}
}
}

// Bank
export function MsgSend(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Better export these functions from the library instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, but they are not currently exported from the library. I can do this at a later time.

@colw colw marked this pull request as ready for review November 28, 2019 16:34
@colw colw requested a review from jbibla as a code owner November 28, 2019 16:34
@faboweb
Copy link
Collaborator Author

faboweb commented Nov 29, 2019

@colw maybe add some coverage to ActionManager?

this.gasEstimate = await this.actionManager.simulate(memo)
} else {
this.gasEstimate = await this.actionManager.simulateTxAPI(
this.createContext(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does this do @colw?

Copy link
Contributor

Choose a reason for hiding this comment

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

The old functionality is still there, and you can opt-in to the transaction service by setting an environment variable when building the front end.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

he meant the context maybe. that is like a bunch of variables you need for the tx to be build

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, that's what i meant. but i don't see the function anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's around, line ~500. Not part of this PR though.

@jbibla
Copy link
Collaborator

jbibla commented Nov 29, 2019

action modal is so large!

@colw
Copy link
Contributor

colw commented Nov 29, 2019

action modal is so large!

What do you mean? Or rather, in which way?

@jbibla
Copy link
Collaborator

jbibla commented Dec 3, 2019

What do you mean? Or rather, in which way?

so many lines!

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.

3 participants