-
Notifications
You must be signed in to change notification settings - Fork 117
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
multi: add address version to database #509
Conversation
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.
@guggero I don't think the asset version field was removed in https://github.com/lightninglabs/taproot-assets/pull/501/files#diff-ccfff66eef18bce3fec0f292a24a84b7163361f1af5090d00b6139fdf8115479R102
We just added a new field for the Tap version. The AssetVersion field was not removed or replaced.
I can see that this PR bumps all of the Tap TLV values. I think we should note that in the commit.
Ah yes, you're right, sorry. But I guess we didn't end up storing the address version in the DB. Updated the PR description to mention the TLV values. |
Yes, true! |
@guggero I think you still need to update the commit message. |
This commit adds the recently added address version to the database as well.
Right, fixed. |
6f7c857
to
33f722e
Compare
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.
Looks good to me! 👍
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.
LGTM 🦚
With the recent PR #501 we changed the asset version within an address into the address' version itself. We still need the asset version in the address as well though, so this commit restores it, including the database storage.
EDIT: This re-orders all taproot address TLV field values to get them nicely incrementing. I'll follow up with a PR to the BIP.