Skip to content

feat: new offer uuid and fingerprint#292

Merged
sedited merged 3 commits intofarcaster-project:mainfrom
h4sh3d:feat/offer-uuid
Aug 14, 2022
Merged

feat: new offer uuid and fingerprint#292
sedited merged 3 commits intofarcaster-project:mainfrom
h4sh3d:feat/offer-uuid

Conversation

@h4sh3d
Copy link
Member

@h4sh3d h4sh3d commented Aug 10, 2022

Switch behavior from offer id and public offer id computed as hash of serialized content to standalone uuid field in Offer type.

A new function .fingerprint() is introduced to match the previous behavior of .id().

@h4sh3d h4sh3d requested a review from sedited August 10, 2022 13:29
Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

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

Very nice change :)

PublicOffer<bitcoin::Amount, monero::Amount, CSVTimelock, SatPerVByte>,
consensus::Error,
> = deserialize(&hex::decode(invalid).unwrap()[..]);
> = deserialize(&hex::decode(inval).unwrap()[..]);
Copy link
Contributor

Choose a reason for hiding this comment

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

invalid?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was to keep the same number of chars as 'valid' for the alignment :)

Done in dae4459

@sedited
Copy link
Contributor

sedited commented Aug 10, 2022

Is there something we can do to get strict encoding for the UUID alone? Maybe wrap it in a struct?

@h4sh3d
Copy link
Member Author

h4sh3d commented Aug 12, 2022

Is there something we can do to get strict encoding for the UUID alone? Maybe wrap it in a struct?

I thought about it. It is not the nicest but Uuid is fully compatible with [u8; 16], so I'd try using that instead when strict-encoding is needed, or wrap it in node. I don't have a better solution yet.

@h4sh3d h4sh3d requested a review from sedited August 12, 2022 15:18
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.

2 participants