-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
add statechange for owned-fraction in rgb21 invoice data #227
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #227 +/- ##
========================================
- Coverage 17.6% 17.5% -0.1%
========================================
Files 37 37
Lines 7535 7557 +22
========================================
Hits 1323 1323
- Misses 6212 6234 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
I think it is always nice to have compiler guarantees instead of unreachable
- pls see my comments
src/interface/contract.rs
Outdated
*self = match self { | ||
OwnedFractionChange::Dec(neg) => OwnedFractionChange::Dec(*neg + sub), | ||
OwnedFractionChange::Zero => OwnedFractionChange::Dec(sub), | ||
OwnedFractionChange::Inc(pos) if *pos > sub => OwnedFractionChange::Inc(*pos - sub), | ||
OwnedFractionChange::Inc(pos) if *pos == sub => OwnedFractionChange::Zero, | ||
OwnedFractionChange::Inc(pos) if *pos < sub => OwnedFractionChange::Dec(sub - *pos), | ||
OwnedFractionChange::Inc(_) => unreachable!(), | ||
}; |
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.
with Ord
you can get rid of unreachable
and instead enjoy compiler-provided exhaustive security for matches:
*self = match self { | |
OwnedFractionChange::Dec(neg) => OwnedFractionChange::Dec(*neg + sub), | |
OwnedFractionChange::Zero => OwnedFractionChange::Dec(sub), | |
OwnedFractionChange::Inc(pos) if *pos > sub => OwnedFractionChange::Inc(*pos - sub), | |
OwnedFractionChange::Inc(pos) if *pos == sub => OwnedFractionChange::Zero, | |
OwnedFractionChange::Inc(pos) if *pos < sub => OwnedFractionChange::Dec(sub - *pos), | |
OwnedFractionChange::Inc(_) => unreachable!(), | |
}; | |
*self = match self { | |
OwnedFractionChange::Dec(neg) => OwnedFractionChange::Dec(*neg + sub), | |
OwnedFractionChange::Zero => OwnedFractionChange::Dec(sub), | |
OwnedFractionChange::Inc(pos) => match sub.cmp(pos) { | |
Ordering::Less => OwnedFractionChange::Inc(*pos - sub), | |
Ordering::Equal => OwnedFractionChange::Zero, | |
Ordering::Greater => OwnedFractionChange::Dec(sub - *pos), | |
}, | |
}; |
src/interface/contract.rs
Outdated
OwnedFractionChange::Dec(neg) if *neg < add => OwnedFractionChange::Inc(add - *neg), | ||
OwnedFractionChange::Dec(_) => unreachable!(), | ||
OwnedFractionChange::Dec(neg) => match add.cmp(neg) { | ||
Ordering::Greater => OwnedFractionChange::Dec(*neg - add), |
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.
This must be less, not greater - the order of comparison is reversed in the match
23d5530
to
eab82b7
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.
Background:
Now there are
Amount
, andOwnedfraction
in the RGB invoice module, that ispub struct Amount( u64);
andpub struct OwnedFraction(u64);
. We can perform a state change operation for the amount type when we want to build atransfer
transition, and there is also thecheck_add
method for the amount type.Both the
StateChange
andcheck_add
for the amount type can perform algorithm operation(+, -).Then for the rgb21 invoice, there is Ownedfraction which is
u64
similar toAmount
, and now if we want to build atransfer
operation for the rgb21 contract, we just push all of them(owned fraction, token index) into the data state. So I think maybe there is also a state change and acheck_add
method requirement for theOwnedfraction
type, and then the user can add an owned fraction when they want to transfer. Finally, they can push (changed owned fraction, token index) into a data state.Description:
impl StateChange for Ownedfraction