Page MenuHomePhabricator

Fix minerfund in GBT when cashaddr is disabled
ClosedPublic

Authored by jasonbcox on Tue, Apr 13, 22:12.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC4b89a7a531f9: Fix minerfund in GBT when cashaddr is disabled
Summary

When testing changes to https://github.com/viabtc/viabtc_mining_server I noticed that -usecashaddr=0 did
not impact the address format for the miner fund in getblocktemplate. This patch fixes that.

While it would be ideal to move away from legacy address support entirely, it's still very common that mining pool software only support the legacy format. Some reasons to not push for cashaddr-only at this time:

  1. Pushing BCH-prefixed cashaddr doesn't make sense strategically at this time. Once the rebranding is completed, we can put energy behind a rebranded prefix cashaddr.
  2. Pools often mine to the same (legacy) address across different chains, so supporting legacy addresses for the time being is important to them to prevent over-complicating their pool configurations.
Test Plan
ninja check check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Wed, Apr 14, 12:28
deadalnix added a subscriber: deadalnix.

Wouldn't moving away imply to not do the translation in more and more places over time?

Is that causing an actual problem for that mining software?

This revision now requires changes to proceed.Wed, Apr 14, 12:29

Wouldn't moving away imply to not do the translation in more and more places over time?

Is that causing an actual problem for that mining software?

It not strictly causing an issue for the pool software. It only makes it more challenging to configure.

IMO this patch doesn't add significant maintenance on its own, but I understand your stance on this. It creeps. We can table it for now.

jasonbcox edited the summary of this revision. (Show Details)

Reopening this diff. See improved rationale in the summary.

This revision now requires changes to proceed.Wed, Apr 21, 18:00
jasonbcox edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Mon, Apr 26, 21:28