v0.11 fixes #111
v0.11 fixes #111
Conversation
seals/src/txout/blind.rs
Outdated
| BlindSeal::with_rng(CloseMethod::OpretFirst, txid, vout, &mut thread_rng()) | ||
| /// Creates new seal for the provided outpoint. Uses `thread_rng` to initialize blinding | ||
| /// factor. | ||
| pub fn rand(txid: impl Into<Id>, vout: impl Into<Vout>) -> Self { |
There was a problem hiding this comment.
This seems to be the same as we have just below in line 141 new_random
There was a problem hiding this comment.
Good catch. I'll keep new_random and drop rand since the former is used in several places
seals/src/txout/blind.rs
Outdated
| impl<Id: SealTxid> BlindSeal<Id> { | ||
| /// Creates new seal for the provided outpoint. Uses `thread_rng` to initialize blinding | ||
| /// factor. | ||
| pub fn rand_from(outpoint: Outpoint) -> Self { BlindSeal::rand(outpoint.txid, outpoint.vout) } |
There was a problem hiding this comment.
According to rust naming conventions it is better to name the method from_rand
| ChainBlindSeal::<CloseMethod>::from_str( | ||
| "tapret1st:646ca5c1062619e2a2d60771c9dfd820551fb773e4dc8c4ed67965a8d1fae839:1" | ||
| ChainBlindSeal::from_str( | ||
| "646ca5c1062619e2a2d60771c9dfd820551fb773e4dc8c4ed67965a8d1fae839:1" |
There was a problem hiding this comment.
The problem with this seal serialization format is that it is nohow different from Outpoint and easy to mix.
I think we need a prefix anyway, like txoseal: or something
|
|
||
| @mnemonic(chicago-europe-phantom) | ||
| data ExplicitSealTxid : txid Bitcoin.Txid, vout Bitcoin.Vout |
There was a problem hiding this comment.
This type becomes strictly equal to Outpoint. I do not think we need it anymore
There was a problem hiding this comment.
I think we do need it because ExplicitSeal can be ExplicitSeal<Txid> or ExplicitSeal<TxPtr>. Or do you think we could drop the generic from ExplicitSeal?
There was a problem hiding this comment.
I think we can drop generic form and just implement the trait we need (something like TxoSeal... there were some traits for explicit seals) and have ExplicitSeal<TxPtr> renamed into GraphSeal (which is a type alias to it anyway, so we do not need both) - and ExplicitSeal<Txid> (with its alias GenesisSeal) removed, and Outpoint uses instead of them.
We can make things even more simple: in v0.12 I got rid of TxPtr taking convention that Txid of zeros means witness output-based seal. This reduces a lot of code here, at no risk cost (from my experience, each time you have an enum in core, add 1000 lines at least where you deal with it's matches....)
So, if you follow v0.12 approach, you will end up with having just Outpoint as a seal definition - and that's it.
(To be fully frank, in v0.12 we do have just one seal type, TxoSeal, but not Outpoint, since there we support failback seal definition. Not sure we need that in v0.11, which is temporary anyway).
There was a problem hiding this comment.
I think here I can say the same I said in RGB-WG/rgb-core#285 (comment): since this is just refactor I'll do this in another set of PRs after these ones are ACKed
This PR is part of a v0.11 fixes epic, see RGB-WG/rgb-tests#30 for an overview.