-
Notifications
You must be signed in to change notification settings - Fork 187
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
Coax1d digital cash solution #8
base: solutions
Are you sure you want to change the base?
Conversation
…f/blockchain-from-scratch into coax1d-digital-cash-exercise
} | ||
|
||
fn handle_transfer(starting_state: State, spends: Vec<Bill>, receives: Vec<Bill>) -> State { | ||
if spends.is_empty() || receives.is_empty() { |
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.
It should be okay if there are no receives. This is how you burn.
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.
kk
let mut next_state = starting_state.clone(); | ||
|
||
{ | ||
let input_set: BTreeSet<_> = spends.iter().collect(); |
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.
Why not just just the same HashMap
that we already use
d?
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 is true. I was just rushing trying to get this out asap.
if total_input.checked_add(amount).is_none() { | ||
return starting_state; | ||
} |
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 is checking for an overflow right? I wonder if we should add a test to make sure this is handled correctly. Or if we want to keep this simple and not worry about overflow, you could just do normal addition here.
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.
Ya I can add one. Thanks for the catch
if output.serial != next_state.next_serial { | ||
return starting_state; | ||
} |
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 made me realize that I didn't design the problem in the nicest way for the user. It would be nice if we didn't require the user to provide the next serial number but rather we just made them provide the owner and amount, then fill the correct serial here. Maybe too big of a change at this point, but figured I'd share the thought.
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 am going to say for next iteration we can make it simpler. I just dont have time to continue on this have to move on to the other stuff. I am almost finished with the p2p lec now.
@@ -28,7 +28,7 @@ pub trait StateMachine { | |||
} | |||
|
|||
/// A set of play users for experimenting with the multi-user state machines | |||
#[derive(Hash, Eq, PartialEq, Debug, Clone)] | |||
#[derive(Hash, Eq, PartialEq, Debug, Clone, Ord, PartialOrd)] |
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 we also won't need these traits if we use a HashMap for deduplicating
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.
true
name says it all