-
Notifications
You must be signed in to change notification settings - Fork 89
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
Feature/zap #609
Feature/zap #609
Conversation
@gerlofvanek @AllienWorks what needs to be done here :
|
|
@anandsinghparihar @vikas-cis can any of you please implement these changes please?
I'll add the rest of the stuff then, style it etc. Btw @pciavald good thinking about TX cost. We don't have that yet, in the GUI, at all! This needs to be done as well. |
@AllienWorks I am on it |
@AllienWorks please check |
this.log.d('zap sendtypeto simulate', info); | ||
|
||
// TODO: ask user to confirm info.fee in a modal | ||
this.dialog.open(ZapWalletsettingsComponent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anandsinghparihar can you pass the info.fee
to the dialog here please ?
|
||
private log: any = Log.create('zap-walletsettings'); | ||
|
||
fee: number = 42; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in order to assign it here
@pciavald request changes pushed !! |
@pciavald I am fixing |
I've tried this on mainnet, wanted to zap the few remaining %.. Zapped two times, two TX went away, but the progress haven't moved at all. |
@AllienWorks zapped two times ? shouldn't be possible :/ |
@pciavald I've zapped once first, a balance transfer TX appeared, but the progress % stayed the same, so I waited a few blocks and when still nothing happened, I've zapped for the second time. I was running this branch via |
@pciavald after our session, my testnet wallet (which is not even encrypted) doesn't display the CS widget: I know it's not a standard behaviour (having your wallet unencrypted), but this should be solved anyway – some of the users might be affected by this (at least the newcomers indeed). I propose having the same style of the widget as "unlock to check state" but with a message like "encrypt wallet to setup cold staking" or something like that (with button pointing to wallet encryption modal ofc). Edit: if you insert the logic, I can style/write it myself, np. |
@pciavald test cases is fixed |
this still has a bug, @AllienWorks reported wrong zapping logic when zapping on several utxos. We need to investigate this. |
Pull Request Test Coverage Report for Build 3035
💛 - Coveralls |
Ability to ZAP the remaining balance not in cold staking yet.