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

ui: fee info and best unit functions #2762

Merged
merged 9 commits into from
Jun 4, 2024
Merged

Conversation

buck54321
Copy link
Member

@buck54321 buck54321 commented May 5, 2024

Adds a dialog to display fee information for a blockchain.

image

Adds Doc methods for selecting the most appropriate unit for a value. Adds a standardized element set that allows quickly setting values with units or rates with units. Adds hover menu to allow user to select different units for a displayed value. Populates alternative units in various UnitInfo.

Copy link
Contributor

@ukane-philemon ukane-philemon left a comment

Choose a reason for hiding this comment

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

I'm running this PR but can't see the fee box. Do you know if there is a setting I need?

client/asset/zec/zec.go Show resolved Hide resolved
client/core/trade.go Outdated Show resolved Hide resolved
client/core/wallet.go Show resolved Hide resolved
@buck54321
Copy link
Member Author

buck54321 commented May 5, 2024

I'm running this PR but can't see the fee box. Do you know if there is a setting I need?

It's only displayed if you have both fiat exchange rate and a fee rate from the wallet. If you're on an SPV wallet, you must have external rates allowed (probably all utxo wallets on simnet, actually). You also must have fiat exchange rates on.

All of these should be on by default now, but if you're not starting with a fresh wallet, they might not be.

@ukane-philemon
Copy link
Contributor

If you're on an SPV wallet, you must have external rates allowed (probably all utxo wallets on simnet, actually).

I'm running testnet and all these settings are on 🥲 but it's still not showing up.

@buck54321
Copy link
Member Author

I'll hit you on Matrix

@ukane-philemon
Copy link
Contributor

ukane-philemon commented May 5, 2024

I'll hit you on Matrix

Thanks! Works now, I wasn't running the correct exec.

I see this weird behaviour. The popup refused to close and I could open more and more on top of the previous popup.
EDIT: I refreshed and am now unable to reproduce. I dunno what went wrong.

Screenshot 2024-05-06 at 12 28 36 AM

@buck54321
Copy link
Member Author

I had a couple of bugs with the hovering selection menu. Should be fixed now. Also have fixed a wallet configuration bug that was preventing external fiat rates from being turned on for wallets created in the quick setup form. Made external rate fallback default true for all assets.

@ukane-philemon
Copy link
Contributor

feeState has always been null for btc. Do I need to author a tx before it shows up? I can only see Unable to get optimal fee rate, using fallback of 4539978304 log.

Testing well for dcr.

Copy link
Contributor

@ukane-philemon ukane-philemon left a comment

Choose a reason for hiding this comment

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

Looking good, but still not able to view the fee box for btc and bch.

@JoeGruffins
Copy link
Member

I see the fee boxes. @ukane-philemon are you sure your site files are on the right commit? There's a Build ID at the bottom of the Settings menu you can check to make sure:

image

@ukane-philemon
Copy link
Contributor

I had to toggle the external fee option, reconfigure and restart before it showed up for btc.

2024-05-07 11:17:40.506 [DBG] CORE[dcr]: tip change: 1382877 (ce80dfca666f121f63b6d246c4505f4ed66909ad2a069055caca007ea53fa380) => 1382878 (e283aa45893b0d72bcca3f55ca2033ea08812b9c96e25186efdc291f36641170)
2024-05-07 11:17:40.528 [DBG] CORE[dcr][SPV]: Starting cspp funds mixer with mix.decred.org:15760
2024-05-07 11:18:46.752 [INF] CORE: notify: |SUCCESS| (walletconfig) Wallet configuration updated - Configuration for btc wallet has been updated. Deposit address = tb1qvztsnndk2tghkueedvnvxv8c4kenm8kxu2g9m7
2024-05-07 11:18:46.752 [INF] CORE: btc wallet configuration updated without a restart 👍

Screenshot 2024-05-07 at 12 21 54 PM Screenshot 2024-05-07 at 12 22 14 PM

@ukane-philemon
Copy link
Contributor

ukane-philemon commented May 7, 2024

I'm yet to reconfigure BCH, It's still in the previous state as BTC. For some reasons, it seems the apifeefallback is not properly populated until a reconfigure.

Note: I haven't used this testnet wallet for months. What changed?

It turns out the settings passed to the wallet do not include apifeefallback(map[special_activelyUsed:false]) but it is checked on the UI.

EDIT: I created a new wallet and it works flawlessly. I didn't have to reconfigure.

Screenshot 2024-05-07 at 1 28 25 PM Screenshot 2024-05-07 at 1 28 50 PM

Older wallets "might" need to reconfigure.

Screenshot 2024-05-07 at 12 24 17 PM

dex/networks/eth/params.go Outdated Show resolved Hide resolved
dex/networks/eth/params.go Outdated Show resolved Hide resolved
@ukane-philemon
Copy link
Contributor

@buck54321 can we use Errorf for the log in func (btc *baseWallet) FeeRate() uint64? Maybe you can add to your PR in your next commit.

Copy link
Contributor

@martonp martonp left a comment

Choose a reason for hiding this comment

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

Looks good, I just don't see the fees displayed for DCR. Maybe a UI issue since DCR has additional panels there?

client/asset/interface.go Outdated Show resolved Hide resolved
client/asset/eth/eth.go Outdated Show resolved Hide resolved
@buck54321
Copy link
Member Author

buck54321 commented May 22, 2024

func (btc *baseWallet) FeeRate() uint64

We can't do it at an error because it's not necessarily an error. For an spv wallet or an rpc wallet without a primed estimatesmartfee and with external fee sources disabled, the inability to get a fee rate ourselves is ok.

@buck54321 buck54321 merged commit c47de28 into decred:master Jun 4, 2024
5 checks passed
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

4 participants