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

set opret/tapret host if needed #46

Merged
merged 1 commit into from
May 10, 2023

Conversation

nicbus
Copy link
Member

@nicbus nicbus commented May 8, 2023

This PR adds support for automatically setting opret/tapret host if needed.

The implementation is similar to my previous rgb-std PRs #14 and #15.

Note: I made the opret and tapret modules from psbt public (and not just pub(crate)) as they're also useful outside of rgb-wallet (e.g. we need to use opret in rgb-lib).

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.

This will wipe the first OP_RETURN if it is present in the tx (doe to some reason) and will force the first tapret to host the commitment, even if it was not intendend. Why we do not leave marking the proper outputs to the wallet devs?

@nicbus nicbus force-pushed the pay_set_commitment_host branch from b654087 to 79cd55a Compare May 9, 2023 14:29
@nicbus
Copy link
Member Author

nicbus commented May 9, 2023

This will wipe the first OP_RETURN if it is present in the tx (doe to some reason) and will force the first tapret to host the commitment, even if it was not intendend. Why we do not leave marking the proper outputs to the wallet devs?

Doing so from a rust piece of software is not an issue, provided opret/tapret modules are public.
Using the CLI only, on the other hand, I don't see a way to do the same, without adding an ad-hoc tool to prepare the PSBT to host the commitment.

Taking that into consideration, I think the best approach is thus to:

  • modify this PR to only make opret/tapret modules public
  • open a PR on RGB-WG/rgb to add a command that only sets opret/tapret, so it can be called only if needed

If you agree I'll be glad to proceed this way.

@dr-orlovsky
Copy link
Member

Yes, I agree that this is the best approach

@nicbus nicbus force-pushed the pay_set_commitment_host branch from 79cd55a to 0d395e1 Compare May 9, 2023 19:47
@nicbus
Copy link
Member Author

nicbus commented May 9, 2023

Yes, I agree that this is the best approach

PR updated dropping automatic setting of commitment host and only making the required modules public.

@dr-orlovsky dr-orlovsky added the enhancement New feature or request label May 10, 2023
@dr-orlovsky dr-orlovsky added this to the v0.10.x milestone May 10, 2023
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 0d395e1

@dr-orlovsky dr-orlovsky merged commit 7501ed2 into RGB-WG:master May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants