Skip to content

Conversation

@coax1d
Copy link
Collaborator

@coax1d coax1d commented Jan 15, 2023

name says it all

@coax1d coax1d requested a review from JoshOrndorff January 15, 2023 18:12
@coax1d coax1d self-assigned this Jan 15, 2023
}

fn handle_transfer(starting_state: State, spends: Vec<Bill>, receives: Vec<Bill>) -> State {
if spends.is_empty() || receives.is_empty() {
Copy link
Owner

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.

Copy link
Collaborator Author

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();
Copy link
Owner

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 used?

Copy link
Collaborator Author

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.

Comment on lines +157 to +159
if total_input.checked_add(amount).is_none() {
return starting_state;
}
Copy link
Owner

@JoshOrndorff JoshOrndorff Jan 15, 2023

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.

Copy link
Collaborator Author

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

Comment on lines +202 to +204
if output.serial != next_state.next_serial {
return starting_state;
}
Copy link
Owner

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.

Copy link
Collaborator Author

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.


/// 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)]
Copy link
Owner

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants