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

Backport upstream bech32m implementation & implement converttex RPC method. #6891

Merged
merged 10 commits into from
May 16, 2024

Conversation

nuttycom
Copy link
Contributor

@nuttycom nuttycom commented May 16, 2024

Includes changes cherry-picked from the following upstream PRs:

@nuttycom nuttycom force-pushed the converttex branch 2 times, most recently from 9fb1d00 to 13b4cbe Compare May 16, 2024 17:16
murrayn and others added 8 commits May 16, 2024 11:29
Added are:

* Vector(arg1,arg2,arg3,...) constructs a vector with the specified
  arguments as elements. The vector's type is derived from the
  arguments. If some of the arguments are rvalue references, they
  will be moved into place rather than copied (which can't be achieved
  using list initialization).

* Cat(vector1,vector2) returns a concatenation of the two vectors,
  efficiently moving elements when relevant.

Vector generalizes (and replaces) the Singleton function in
src/descriptor.cpp, and Cat replaces the Cat function in bech32.cpp

(cherry picked from commit bitcoin/bitcoin@e65e61c)
(cherry picked from commit bitcoin/bitcoin@0334602)
@str4d str4d added C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. A-rpc-interface Area: RPC interface labels May 16, 2024
@str4d str4d added this to the Release 5.10.0 milestone May 16, 2024
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Concept ACK 90888c6. Documentation fixes are blocking, the rest is a soft block (can be overridden by another reviewer disagreeing with me).

src/key_constants.h Show resolved Hide resolved
src/chainparams.cpp Outdated Show resolved Hide resolved
src/key_io.cpp Outdated Show resolved Hide resolved
src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
Comment on lines +32 to +48
test_vectors = [
["tmQqjg2hqn5XMK9v1wtueg1CpzGbgTNGZQu", "texregtest15hhh9uprfp6krumprglg6qpx3928ulrnlxt9na"],
["tmGiqpWKPJdraF2PqBzPojzkRbDE4fPTyAF", "texregtest1fns2jk8xpjr7rqtaggn2zpmcdtfyj2jer8arm0"],
["tmEkTF6UovNsEQM9h1ehnA3byw6yhFCJWor", "texregtest1xulx2a0pgc84phkdtue67zwe26axtcvvyaf6yu"],
["tmGoyC4XZ1GNCdJGk96K6mT8jxDQEhzVbfR", "texregtest1fhvw29vvg37mep5kkhyew47rrqadjtyk4xzx8n"],
["tmG4gSmUZzCcyR6S5nBhEFrfmodmUjXXAZG", "texregtest1gk5swlnzf8m5hc9x82344aqv5s90k5x2dvqyvh"],
["tmKgnRCv6SjwEFgXhqPoADKp3HLFF67Seww", "texregtest1d4j4uz8wnl5zmuzdl7y3fykrkk2zarnccfs578"],
["tmTymg9bGECw8tR8WHepE45c4joNTUt1zth", "texregtest1epwdxsm94e9wh2zwad7j885gelxly2f8d4mqry"],
["tmMGGBngBJwgTYWCx23zWDY7QvLateZNqCC", "texregtest106exvc6ufugdwppy7vnuf2vwztf5qh9r4tpsfa"],
["tmUyukTGWjTM7Nw4j8zgbZZwPfM8enu9NvZ", "texregtest16ddmp690el6vrzajhftc3fqpmx4a3cgfqf97yn"],
["tmDsJSojZxU3sb3LGMs6nMC1SVSQhKD99My", "texregtest19kgadp7hwu08xlufnvr2r0p5aygw3ss60px28u"],
["tmWezycaJjPoXNJxK2zvzELjS65mzExnvVE", "texregtest1ukuxwrfrj20q65c25ssk3nkathsy8qneehv5vl"],
["tmMUEbcXX7tVwtRjaSaMVfzXu6PCeyMnsCH", "texregtest1srmppx752ux07mntsjmthpddy2enunfuh6mlej"],
["tmDjbRj8go7BrS8AxSjfjTBsCcHw7J45SCi", "texregtest19sw2lplpdswc4zvdtz3z8yt42wtz4recz5plym"],
["tmSSJADGK9bgafaY9eih17WRazi3KzNL7RT", "texregtest1kacdt4amhphqx6tf4gla7a4qg8h9ey5tauz24v"],
["tmSJ3JSRx2R71MfBksWzHNEfMxUgzMxDMxy", "texregtest1khs34j6m944un75ula8xxgvgdnjc4m2l0kfgpy"]
];
Copy link
Contributor

Choose a reason for hiding this comment

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

I did not check these.

src/rpc/common.h Outdated Show resolved Hide resolved
Co-authored-by: str4d <thestr4d@gmail.com>
str4d
str4d previously approved these changes May 16, 2024
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK 069ed75. Should be mentioned in the release notes, but that doesn't have to happen in this PR.

Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Re-utACK fa3af5f

@nuttycom nuttycom merged commit 77c5cfe into zcash:master May 16, 2024
140 of 149 checks passed
@nuttycom nuttycom deleted the converttex branch May 16, 2024 22:20
if (c >= 'a' && c <= 'z') lower = true;
if (c >= 'A' && c <= 'Z') upper = true;
else if (c >= 'A' && c <= 'Z') upper = true;
else if (c < 33 || c > 126) return {};
}
if (lower && upper) return {};
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't currently expose any way to decode a TEX address from zcashd, but if we did, it would be necessary to check that all-uppercase addresses are accepted (but not generated). The test vectors don't have any examples of all-uppercase addresses. Note that they may occur in practice because an all-uppercase alphanumeric string is more efficiently represented in a QR code.

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

Post-hoc ACK, with a comment.

@str4d str4d modified the milestones: Release 5.10.0, Release 5.9.1 May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc-interface Area: RPC interface C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants