Skip to content

Cli: send encoded funding info#751

Merged
h4sh3d merged 3 commits into
farcaster-project:mainfrom
sedited:needsFundingSerialization
Nov 29, 2022
Merged

Cli: send encoded funding info#751
h4sh3d merged 3 commits into
farcaster-project:mainfrom
sedited:needsFundingSerialization

Conversation

@sedited

@sedited sedited commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

This is required to extract the funding info once the complete grpc support has landed.

We can now also choose what is displayed in the cli. Which one do you prefer?

YAML:

---
funding_infos:
  - Bitcoin:
      swap_id: "0xaac3ae5727b11e04eaf44a7e96a275e23d39bf989bcc00e6fd21975e9d8b363e"
      address: tb1q994a2m33suqpyj9ewn8zqnr0lx0htt4fcgk3pa
      amount: 0.00001473
  - Bitcoin:
      swap_id: "0x2b446babde3d4cda0f47e79759ed797ec8dfcc270e43ad27cd4198aee2d2348d"
      address: tb1q29rd6at9emc4jwttd9umvzwuyks2mwhjnc5844
      amount: 0.00001473

String (this PR):

0xaac3ae5727b11e04eaf44a7e96a275e23d39bf989bcc00e6fd21975e9d8b363e needs 0.00001473 BTC to tb1q994a2m33suqpyj9ewn8zqnr0lx0htt4fcgk3pa
0x2b446babde3d4cda0f47e79759ed797ec8dfcc270e43ad27cd4198aee2d2348d needs 0.00001473 BTC to tb1q29rd6at9emc4jwttd9umvzwuyks2mwhjnc5844

@sedited sedited added the enhancement New feature or request label Nov 3, 2022
@sedited sedited force-pushed the needsFundingSerialization branch from fb41735 to 9f415d1 Compare November 3, 2022 17:22
@h4sh3d

h4sh3d commented Nov 4, 2022

Copy link
Copy Markdown
Member

All other cli call return YAML (only progress --follow doesn't bu could be compatible, but does it makes sens IDK).

IMO I'd stick with yaml because it is human readable and machine parse-able (easily with e.g. yq). Maybe changing fundings_infos into something more specific about the NEEDS of funding would be good.

@sedited sedited force-pushed the needsFundingSerialization branch 2 times, most recently from 1bd9016 to f78085a Compare November 4, 2022 16:46
@sedited

sedited commented Nov 4, 2022

Copy link
Copy Markdown
Contributor Author

Alright, I pushed a yaml version now and renamed the funding_infos field.

PSA: This will break all the tooling that is depending on needs funding right now.

@sedited sedited force-pushed the needsFundingSerialization branch from c945a62 to 1864ef5 Compare November 4, 2022 20:52
@sedited sedited linked an issue Nov 6, 2022 that may be closed by this pull request
Make all cli messages serializable #285
Closed
@sedited sedited force-pushed the needsFundingSerialization branch from 1864ef5 to 9180339 Compare November 7, 2022 12:32
@sedited sedited force-pushed the needsFundingSerialization branch from 9180339 to 6ac1d5b Compare November 14, 2022 17:18
@sedited sedited force-pushed the needsFundingSerialization branch from 6ac1d5b to 454b7c4 Compare November 28, 2022 13:46
@h4sh3d h4sh3d force-pushed the needsFundingSerialization branch from 9eb28ea to 58cc95d Compare November 29, 2022 12:05
h4sh3d
h4sh3d approved these changes Nov 29, 2022
View reviewed changes

@h4sh3d h4sh3d left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK

I propose the commit 58cc95d that simplifies a bit the info handling in farcasterd, but we can drop it if wanted.

@sedited

sedited commented Nov 29, 2022

Copy link
Copy Markdown
Contributor Author

ACK 58cc95d

@h4sh3d h4sh3d merged commit aa0f928 into farcaster-project:main Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@h4sh3d h4sh3d h4sh3d approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make all cli messages serializable

2 participants

@sedited @h4sh3d