Changes required by RGB wallet PSBT operations #113
Conversation
dr-orlovsky
added
epic
| impl Display for CoinAmount { | ||
| fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { | ||
| let mut int = self.int.to_string(); | ||
| if f.alternate() { |
There was a problem hiding this comment.
What is this? Why chunks of 3 bytes? Is unsafe really necessary?
There was a problem hiding this comment.
Unsafe is required because of from_utf8_unchecked - since we are sure the string is UTF8
Chunks of three bytes to print the separators: 100'000'000
| #[allow(clippy::result_large_err)] | ||
| impl RgbInvoiceBuilder { | ||
| pub fn new(beneficiary: impl Into<Beneficiary>) -> Self { | ||
| Self(RgbInvoice { |
There was a problem hiding this comment.
I would recommend implementing Default, moving this there, then using Default::default() here.
There was a problem hiding this comment.
With Default you will get an invalid invoice: no transports, no beneficiary; and this will require converting finish to return Result type (and introducing another error type). I am not sure this is better.
| ) -> Result<(), InventoryError<Self::Error>>; | ||
|
|
||
| /// # Safety |
There was a problem hiding this comment.
Can you re-add the docs for the unsafe code? Just indicating why unsafe and under what conditions
There was a problem hiding this comment.
I removed it so it since it will not be seen anyway due to the added doc(hidden) method... I do not know how to hide such things properly and at the same time provide those implementing the trait with enough info...
| @@ -471,20 +462,19 @@ impl Inventory for Stock { | |||
| self.consume_consignment(transfer, resolver, force) | |||
| } | |||
|
|
|||
| fn consume_anchor( | |||
| unsafe fn consume_anchor( | |||
There was a problem hiding this comment.
To prevent it from being called from any place other than the clearly defined single place.
Not a best practice, I know - but it is easier not to mistake/miss invalid calls in this way
Co-authored-by: Armando CD <crisdut@users.noreply.github.com>
dr-orlovsky
changed the title
dr-orlovsky
changed the title
dr-orlovsky
changed the title
Also includes support for combined tapret/opret commitments.
Mostly ready - will mark for review the moment wallet library will produce PSBTs.