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

Support to Regtest Address #63

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Support to Regtest Address #63

merged 1 commit into from
Nov 28, 2022

Conversation

crisdut
Copy link
Member

@crisdut crisdut commented Nov 24, 2022

@crisdut crisdut changed the title Support Regtest Prefix Support to Regtest Address Nov 24, 2022
dr-orlovsky
dr-orlovsky requested changes Nov 24, 2022
View reviewed changes
src/bin/btc-cold.rs Outdated Show resolved Hide resolved
@crisdut crisdut force-pushed the feat/regtest-support branch from 4874880 to 484ff41 Compare November 24, 2022 07:06
@crisdut crisdut requested a review from dr-orlovsky November 24, 2022 07:06
dr-orlovsky
dr-orlovsky requested changes Nov 24, 2022
View reviewed changes
Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

What I meant is the modification of the AddressCompat structure to support regtest network. It should be continued to be used throughout the code.

src/bin/btc-cold.rs Outdated Show resolved Hide resolved
src/bin/btc-cold.rs Outdated Show resolved Hide resolved
src/bin/btc-cold.rs Show resolved Hide resolved
@dr-orlovsky
Copy link
Member

dr-orlovsky commented Nov 24, 2022 •
edited
Loading

Sorry, I missed that you did the PR updating AddrCompat. But after research I found out that it can't work this way. Gave detailed explanations there: #64 (comment)

@crisdut crisdut force-pushed the feat/regtest-support branch from 484ff41 to f549d85 Compare November 26, 2022 14:34
@crisdut crisdut requested a review from dr-orlovsky November 26, 2022 14:46
@dr-orlovsky dr-orlovsky added the bug Something isn't working label Nov 26, 2022
@dr-orlovsky dr-orlovsky added this to the v0.8.x milestone Nov 26, 2022
dr-orlovsky
dr-orlovsky requested changes Nov 26, 2022
View reviewed changes
Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

One small change and it can be merged. Thank you!

src/bin/btc-cold.rs Outdated Show resolved Hide resolved
@crisdut crisdut force-pushed the feat/regtest-support branch from f549d85 to c52c1db Compare November 27, 2022 17:50
@crisdut crisdut requested a review from dr-orlovsky November 27, 2022 17:55
@dr-orlovsky
Copy link
Member

I am really sorry, but I still see at least two unwraps in the code :(

@crisdut crisdut force-pushed the feat/regtest-support branch from c52c1db to d037197 Compare November 28, 2022 11:21
@crisdut
Copy link
Member Author

crisdut commented Nov 28, 2022

I am really sorry, but I still see at least two unwraps in the code :(

Oh, I'm sorry, I didn't see there were other.

dr-orlovsky
dr-orlovsky approved these changes Nov 28, 2022
View reviewed changes
Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

ACK d037197 as a temporarily fix pending #65

@dr-orlovsky dr-orlovsky merged commit 05702f3 into BP-WG:v0.8 Nov 28, 2022
@crisdut crisdut deleted the feat/regtest-support branch December 29, 2022 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr-orlovsky dr-orlovsky dr-orlovsky approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
Wallet
Status: Done
Milestone
v0.8.x
Development

Successfully merging this pull request may close these issues.
2 participants
@crisdut @dr-orlovsky