bch: switch to testnet4 and use custom address stringer #1606
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chappjc
commented
May 6, 2022
| // Expiration returns the expiration time of the contract, which is the earliest | ||
| // time that a refund can be issued for an un-redeemed contract. Part of the | ||
| // asset.AuditInfo interface. |
There was a problem hiding this comment.
At one point there was an asset.AuditInfo interface, but it it became a concrete type at one point and these remained.
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| privKey, err := btc.node.privKeyForAddress(addrStr) |
There was a problem hiding this comment.
Not sure, but perhaps bitcoincashd (Bitcoin Cash Node) was able to pull out the pubkey from the bitcoin/legacy addresses and locate the private key. The chain params were the clone params with the correct bch address ID values, but this wasn't making Cash Addresses.
There was a problem hiding this comment.
Yeah, that's what it was doing.
$ ./beta getaddressinfo mgDzcoxdBrhu435knkdUEP5UGVYGKUJJgN
{
"address": "bchreg:qqru9lns98ugq85h4cl3p8srf4y9w8cyt5eux55xmn",
"scriptPubKey": "76a91407c2fe7029f8801e97ae3f109e034d48571f045d88ac",
"ismine": true,
"iswatchonly": false,
"isscript": false,
"pubkey": "0310892969dde0c7f6e177ce0f1a8ba807fbbbbc05135e9c47479f0aa1fa2555a5",
...
|
|
Ah, dang. Hopefully we did things similarly. |
|
We're close. I can minimize conflicts because having this fix separate makes sense IMO, but some points:
|
|
Your way is better. I'll adapt. |
|
Shall I merge this then? |
chappjc
force-pushed
the
bcht4
branch
2 times, most recently
from
29103e5
to
6b3c30c
Compare
|
Or just discard the stringer commit? Whatever |
buck54321
approved these changes
May 15, 2022
|
Any leads on getting some testnet4 BCH? |
site: add bch explorer links
The exchange wallets had an address decoder function for converting a string to a btcutil.Address, which is required for assets like BCH with special address formats. However, it is incorrect to then use the btcutil.Address String method go get a human readable address. This adds a complementary helper function for converting from a btcutil.Address to a correct string. Normally this is simply using the Address as a Stringer, but for BCH it is the new EncodeCashAddress function, which powers the existing RecodeCashAddress. This updates the btc baseWallet methods to use the new stringAddr to achieve the reverse of decodeAddr. This ensures that the string address stored in the fundingCoins is correct for the asset. This also ensures the recipient address in AuditContract is correct. I believe the main issue resolved by this is providing the correct address to the node.privKeyForAddress method when signing. This also provides the correct string to the getaddressinfo RPC. It is not clear if bitcoincashd was previously ok with the incorrect address strings when handling RPCs. This also removes the unused (*auditInfo).Recipient method, and fixes the documentation for the other methods since there is no longer an asset.AuditInfo *interface*, just a struct.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pulled out of a larger Electrum branch.
This does:
baseWalletto resolve what appears to be incorrect conversion ofbtcutil.Addresstypes to strings in places. See commit message below.