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

multi: retire load.WalletLoad #235

Merged
merged 4 commits into from
Dec 1, 2023
Merged

multi: retire load.WalletLoad #235

merged 4 commits into from
Dec 1, 2023

Conversation

itswisdomagain
Copy link
Collaborator

@itswisdomagain itswisdomagain commented Nov 3, 2023 •
edited
Loading

Resolves #229.

@itswisdomagain itswisdomagain force-pushed the kill-walletload branch 2 times, most recently from bee2a57 to e37e87f Compare November 4, 2023 11:29
itswisdomagain
itswisdomagain commented Nov 4, 2023
View reviewed changes
ui/page/root/wallet_list.go Outdated Show resolved Hide resolved
ui/page/settings/settings_page.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Show resolved Hide resolved
@itswisdomagain itswisdomagain force-pushed the kill-walletload branch 4 times, most recently from 95a2bfc to 4a30d1f Compare November 8, 2023 22:43
@itswisdomagain itswisdomagain marked this pull request as ready for review November 8, 2023 22:43
@itswisdomagain itswisdomagain force-pushed the kill-walletload branch 2 times, most recently from 7cf232e to b9b3512 Compare November 10, 2023 18:30
ukane-philemon
ukane-philemon requested changes Nov 10, 2023
View reviewed changes
Copy link
Collaborator

@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.

Looks good.

Got this panic when I visited the politia page:

2023-11-10 21:04:18.616 [ERR] UI: No rate information available
2023-11-10 21:04:19.671 [INF] POLT: Politeia sync: started
panic: interface conversion: wallet.Asset is *ltc.Asset, not *dcr.Asset

goroutine 2159 [running]:
github.com/crypto-power/cryptopower/ui/page/components.LoadPolicies(0x10004611c?, {0x101ac5390?, 0xc003366240?}, {0xc003b6c370?, 0x1007fda25?})
        /Users/philemon/Desktop/osp/cryptopower/ui/page/components/treasury_list.go:101 +0x377
github.com/crypto-power/cryptopower/ui/page/governance.(*TreasuryPage).FetchPolicies.func1()
        /Users/philemon/Desktop/osp/cryptopower/ui/page/governance/treasury_page.go:160 +0x35
created by github.com/crypto-power/cryptopower/ui/page/governance.(*TreasuryPage).FetchPolicies in goroutine 13
        /Users/philemon/Desktop/osp/cryptopower/ui/page/governance/treasury_page.go:159 +0x58
exit status 2

ui/page/components/components.go Outdated Show resolved Hide resolved
ui/page/components/components.go Outdated Show resolved Hide resolved
ui/page/components/consensus_list.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
@itswisdomagain
Copy link
Collaborator Author

Got this panic when I visited the politia page

That appears to be the same error reported in #229 and it happens when visiting the treasury page. Please confirm.

I'll fix the treasury page issue in this PR and close that issue.

Sirmorrison
Sirmorrison suggested changes Nov 20, 2023
View reviewed changes
Copy link
Collaborator

@Sirmorrison Sirmorrison left a comment

Choose a reason for hiding this comment

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

Changes looks straightforward. Just for this one change.

ui/page/components/consensus_list.go Outdated Show resolved Hide resolved
JustinBeBoy
JustinBeBoy requested changes Nov 22, 2023
View reviewed changes
libwallet/assets_config.go Show resolved Hide resolved
libwallet/assets_manager.go Outdated Show resolved Hide resolved
@itswisdomagain itswisdomagain mentioned this pull request Nov 23, 2023
Merged
@itswisdomagain itswisdomagain force-pushed the kill-walletload branch 2 times, most recently from c9eede9 to 078b56e Compare November 28, 2023 02:53
Sirmorrison
Sirmorrison approved these changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@Sirmorrison Sirmorrison left a comment

Choose a reason for hiding this comment

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

LGTM

@dreacot
Copy link
Member

dreacot commented Nov 29, 2023

clicking on a dcr wallet causes a crash

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x44 pc=0x113c1d9]

goroutine 184 [running]:
github.com/crypto-power/cryptopower/ui/cryptomaterial.(*SegmentedControl).SelectedSegment(0xc00cc2b430?)
        /home/user/Projects/src/github.com/cryptopower/cryptopower/ui/cryptomaterial/segmented_control.go:156 +0x39
github.com/crypto-power/cryptopower/ui/page/root.(*SingleWalletMasterPage).OnNavigatedTo(0xc000634000)
        /home/user/Projects/src/github.com/cryptopower/cryptopower/ui/page/root/main_page.go:135 +0x105
github.com/crypto-power/cryptopower/app.(*PageStack).PushAndNavigate(0xc0003a99b0, {0x1effe60, 0xc000634000}, {0x1f02e40, 0xc0006d01f0})
        /home/user/Projects/src/github.com/cryptopower/cryptopower/app/pagestack.go:67 +0x538
github.com/crypto-power/cryptopower/app.(*MasterPage).Display(0xc0006d01f0, {0x1effe60?, 0xc000634000?})
        /home/user/Projects/src/github.com/cryptopower/cryptopower/app/masterpage.go:46 +0x39
github.com/crypto-power/cryptopower/ui/page/root.(*WalletSelectorPage).HandleUserInteractions(0xc0038185a0)
        /home/user/Projects/src/github.com/cryptopower/cryptopower/ui/page/root/wallet_selector_page.go:184 +0x1f0
github.com/crypto-power/cryptopower/ui/page/root.(*HomePage).HandleUserInteractions(0xc003f0f520)
        /home/user/Projects/src/github.com/cryptopower/cryptopower/ui/page/root/home_page.go:180 +0x67
github.com/crypto-power/cryptopower/ui.(*Window).handleFrameEvent(0xc003b8afc0, {{0xc151d34235b0200f, 0x98c0de96ee4, 0x2e145a0}, {0x3f800000, 0x3f800000}, {0x320, 0x28a}, {0x0, 0x0, ...}, ...})
        /home/user/Projects/src/github.com/cryptopower/cryptopower/ui/window.go:224 +0x197
github.com/crypto-power/cryptopower/ui.(*Window).HandleEvents(0xc003b8afc0)
        /home/user/Projects/src/github.com/cryptopower/cryptopower/ui/window.go:195 +0x2b9
main.main.func3()
        /home/user/Projects/src/github.com/cryptopower/cryptopower/main.go:109 +0x1d
created by main.main
        /home/user/Projects/src/github.com/cryptopower/cryptopower/main.go:107 +0x51d
exit status 2

dreacot
dreacot reviewed Nov 29, 2023
View reviewed changes
Copy link
Member

@dreacot dreacot left a comment

Choose a reason for hiding this comment

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

overall i think the changes look good.

however my concerns is in testing app functionality to ensure nothing got broken

ukane-philemon
ukane-philemon approved these changes Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@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.

Looks good.

This PR on MacOS:

Screenshot 2023-11-30 at 5 00 45 PM

@dreacot
Copy link
Member

dreacot commented Nov 30, 2023

Looks good.

This PR on MacOS:

Screenshot 2023-11-30 at 5 00 45 PM

it's reported here #297

@dreacot dreacot merged commit 9aa7f67 into crypto-power:master Dec 1, 2023
1 check passed
@itswisdomagain itswisdomagain deleted the kill-walletload branch December 21, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ukane-philemon ukane-philemon ukane-philemon approved these changes

@dreacot dreacot dreacot approved these changes

@crux25 crux25 Awaiting requested review from crux25

@dmigwi dmigwi Awaiting requested review from dmigwi

@JoeGruffins JoeGruffins Awaiting requested review from JoeGruffins

@Sirmorrison Sirmorrison Awaiting requested review from Sirmorrison

@JustinBeBoy JustinBeBoy Awaiting requested review from JustinBeBoy

Assignees

@itswisdomagain itswisdomagain

Labels
None yet
Projects
dev
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Visiting the treasury page without an creating DCR wallet causes a panic
5 participants
@itswisdomagain @dreacot @JustinBeBoy @Sirmorrison @ukane-philemon