New transfer id workflows #148
Conversation
There was a problem hiding this comment.
Now, the following error is occurring:
The transfer the actual data doesn't match the provided id.\nActual id: transfer:Cs7AAr-dHR89bcS-nJ86BwiN-QXHcMKXj-yRvgKsb3-1D8LnV#baker-manual-patent.\nExpected id: transfer:ARZGSt-jNh7wRYp-zK9cyLXX-o2psW26n-GWf9qedh-j3JxY1#clever-dispute-zipper. presents inconsistency
Just a testing: If we change self.id in the line to self.bindle_id(), the error is corrected.
| @@ -1,11 +1,11 @@ | |||
| -----BEGIN STRICT TYPE LIB----- | |||
| Id: urn:ubideco:stl:GVz4mvYE94aQ9q2HPtV9VuoppcDdduP54BMKffF7YoFH | |||
| Id: GVz4mvYE94aQ9q2HPtV9VuoppcDdduP54BMKffF7YoFH#prince-scarlet-ringo | |||
There was a problem hiding this comment.
Were we supposed to lose the URN prefix?
| -- urn:ubideco:semid:BsYakBZMzKBDfgpfhXt1p81Q1WT27V8U7LvHG4NPmGmM#montana-torso-pierre | ||
| data Error :: supplyMismatch:1 | nonEqualAmounts:2 | invalidProof:3 | insufficientReserves:4 | ||
| | insufficientCoverage:5 | issueExceedsAllowance:6 | ||
| @mnemonic(montana-torso-pierre) |
There was a problem hiding this comment.
What's the reasoning behind providing less data here?
src/containers/transfer.rs
Outdated
| e.commit_to(&self.version); | ||
| e.commit_to(&self.transfer); | ||
|
|
||
| e.commit_to(&self.contract_id()); | ||
| e.commit_to(&self.genesis.disclose_hash()); | ||
| e.commit_to(&TinyOrdSet::from_iter_unsafe( | ||
| self.ifaces.values().map(|pair| pair.iimpl.impl_id()), | ||
| )); |
There was a problem hiding this comment.
All items must be named types.
src/containers/transfer.rs
Outdated
| e.commit_to(&LargeOrdSet::from_iter_unsafe( | ||
| self.bundles.iter().map(|ab| ab.bundle.disclose_hash()), | ||
| )); | ||
| e.commit_to(&LargeOrdSet::from_iter_unsafe( | ||
| self.extensions.iter().map(Extension::disclose_hash), | ||
| )); | ||
| e.commit_to(&SmallOrdSet::from_iter_unsafe(self.terminals_disclose())); | ||
|
|
||
| e.commit_to(&SmallOrdSet::from_iter_unsafe(self.attachments.keys().copied())); | ||
| e.commit_to(&self.supplements); | ||
| e.commit_to(&self.asset_tags); | ||
| e.commit_to(&self.signatures); |
There was a problem hiding this comment.
All items must be named types.
There was a problem hiding this comment.
We have good news: all tests ran successfully in my environment, so I believe that everything is now working.
However, the only bug I found occurs when we try converting a Bindle<Transfer> to string and string to Bindle<Transfer>:
The transfer the actual data doesn't match the provided id.
Actual id: transfer:CeJAm5-nw1WxMzV-81DtaJSa-CTgjJ1MS-g9NmSjvi-6vMkT3#bonanza-shelter-organic.
Expected id: transfer:CgtX9T-9uVVt1Rq-3dPAs62P-CegfHe1Y-Gjs8D8Uu-onnw1L#rebel-justice-lithium. presents inconsistency
I think we need make some changes here, from writeln!(f, "Id: {:-#}", self.id)?; to writeln!(f, "Id: {:-#}", self.bindle_id())?;
If this change, this validation works.
|
@crisdut thank you for figuring out the source of the problem! You are right: Shit, these id's are really getting out of control... |
|
No, I am wrong: id and bindle_id should be the same thing! At least according to the code: rgb-std/src/containers/bindle.rs Line 171 in 5817be7 Any guesses why we have this bug? I can easily fix with your suggestion above, but I am afraid we are missing some deep bug here... |
I found the reason. When we create the bindle, we use the Bindle::new method: pub struct Bindle<C: BindleContent> {
id: C::Id,
data: C,
sigs: TinyVec<Cert>,
}
impl<C: BindleContent> Bindle<C> {
pub fn new(data: C) -> Self {
Bindle {
id: data.bindle_id(),
data,
sigs: empty!(),
}
}
}In some cases, we modify the tranfer, and this changes producing another id when we execute bindle_id and transfer_id, but Bindle::Id remain fixed. Example: https://github.com/RGB-WG/rgb/blob/master/src/pay.rs#L337-L353 My recommendation is remove the directly access by id, and use always bindle_id. What's you think? |
|
@crisdut I did this way in 0a9c92e and updated RGB wallet in master. Please let me know if everything works now - and I will merge this PR |
No description provided.