Skip to content
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

Improve Amount and remove AmountStr. #609

Closed
crodas opened this issue Feb 19, 2025 · 0 comments · Fixed by #612
Closed

Improve Amount and remove AmountStr. #609

crodas opened this issue Feb 19, 2025 · 0 comments · Fixed by #612

Comments

@crodas
Copy link
Contributor

crodas commented Feb 19, 2025

          You can just use the `Amount` there no need to wrap it in the `AmoutnStr`. The reason we have to use `AmountStr` is for when the Keys response is json serialized it needs to be as a string and not an int, so that's really the only place we should use `AmountStr` everywhere else is `Amount`.

EDIT: I see you're creating a Keys which currently does expect the AmountStr

pub fn new(keys: BTreeMap<AmountStr, PublicKey>) -> Self {
. But since whats needed is as i described above we should change this and that can be an amount and we can just create a custom serialize deserilization, that maybe a better way to handle this and then we don't need the AmountStr type?

cc @crodas

Originally posted by @thesimplekid in #605 (comment)

@crodas crodas changed the title You can just use the Amount there no need to wrap it in the AmoutnStr. The reason we have to use AmountStr is for when the Keys response is json serialized it needs to be as a string and not an int, so that's really the only place we should use AmountStr everywhere else is Amount. Improve Amount and remove AmountStr. Feb 19, 2025
crodas added a commit to crodas/cdk that referenced this issue Feb 21, 2025
Fixes cashubtc#609

Instead write a customer serializer for Keys to serialize amounts as strings
@crodas crodas mentioned this issue Feb 21, 2025
2 tasks
crodas added a commit to crodas/cdk that referenced this issue Feb 21, 2025
Fixes cashubtc#609

Instead write a customer serializer for Keys to serialize amounts as strings
crodas added a commit to crodas/cdk that referenced this issue Feb 21, 2025
Fixes cashubtc#609

Instead write a customer serializer for Keys to serialize amounts as strings
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 a pull request may close this issue.

1 participant