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.

Sign up for GitHub

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

Jump to bottom

feat: BCH using chronik #843

Merged
merged 18 commits into from
Aug 14, 2024
Merged

feat: BCH using chronik #843

merged 18 commits into from
Aug 14, 2024

Conversation

chedieck
Copy link
Collaborator

@chedieck chedieck commented Aug 8, 2024 •
edited
Loading

Related to #412

Description

Sets up Chronik for BCH.

Essentially meaning BCH network should now work normally, just as XEC does.

Test plan

Change config

Config should change now, check config/example-config.json. You can remove obsolete variables like grpcClient...URL, chronikClientURL and add the new networkBlockchainURLs variable:

  "networkBlockchainURLs": {
    "ecash": "https://xec.paybutton.io",
    "bitcoincash": "https://chronik.pay2stay.com/bch"
  },

...besides modifying networkBlockchainClients so that it says:

  "networkBlockchainClients": {
    "ecash": "chronik",
    "bitcoincash": "chronik"
  },

Also, remove the property networksUnderMaintenance so it won't inactivate BCH.

Now, after rebooting the containers, bitcoincash should sync and work normally.

Remarks

When working on this issue, I found some issues with Bitcoin Cash in the client.

  • Click-to-copy in the QR code doesn't grab the amount
  • OnSuccess being triggered with a different amount than the one in the button.

I'll create a PR in the client to fix those.

@Klakurka Klakurka self-requested a review August 8, 2024 12:03
Klakurka
Klakurka requested changes Aug 8, 2024
View reviewed changes
Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

  • Update 'Under Maintenance' network message
    image
  • eCash is showing as disconnected for me in /networks
  • Error when visiting /payments and /dashboard:
    image

@Klakurka
Copy link
Member

Also we do already have a task for click-to-copy not working right on BCH: PayButton/paybutton#417

@Klakurka
Copy link
Member

Let's address the BCH success trigger in PayButton/paybutton#403 instead.

@chedieck chedieck force-pushed the feat/bch-chronik branch 2 times, most recently from 32f237c to 7387f6c Compare August 13, 2024 18:01
@chedieck
Copy link
Collaborator Author

Also we do already have a task for click-to-copy not working right on BCH: PayButton/paybutton#417

Alright

Let's address the BCH success trigger in PayButton/paybutton#403 instead.

I don't follow.

  • Update 'Under Maintenance' network message

You have to remove it from your paybutton-config.json. Forgot to mention it in the description, updated.

* eCash is showing as disconnected for me in /networks
* Error when visiting /payments and /dashboard:

Could you confirm you are still getting those now?

@Klakurka
Copy link
Member

I don't follow.

It's a client-side issue so let's just make sure it's working in PayButton/paybutton#403.

Klakurka
Klakurka previously approved these changes Aug 14, 2024
View reviewed changes
Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

Really nice job on this! Works great.

@Klakurka
Copy link
Member

@chedieck Before we merge this, what could we do for tests as I don't see any thing new added for this one.

@chedieck
Copy link
Collaborator Author

@chedieck Before we merge this, what could we do for tests as I don't see any thing new added for this one.

Just added some tests, making sure chronik works for both ecash and bitcoincash.

@Klakurka Klakurka merged commit 4751c1e into master Aug 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Klakurka Klakurka Klakurka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chedieck @Klakurka