Add hashing to secp256k1 #863
Open
skaunov wants to merge 7 commits intoarkworks-rs:masterfrom
Open
Conversation
skaunov
changed the title
secp256k1_XMD:SHA-256_SSWU_RO_ missing the isogenous generatorsecp256k1
skaunov
requested review from
tyshko-rostyslav,
weikengchen and
z-tech
and removed request for
a team
Member
|
(I think the PR has been based on an older branch, but it has been quite a while and synchronizing the code becomes not very possible. I will let Claude to take a look to see how to "rebase" it to be cleaner, but it may take some time..) |
Author
|
The PR itself was quite local/small. I can just remove the commits
rebasing back then added. Would it help?
It also should rely on another PR for that moment, but that's a
separate story.
…On Пн, мар 16 2026 at 10:20:15 -0700, Weikeng Chen ***@***.***> wrote:
*weikengchen*
left a comment
(arkworks-rs/algebra#863)
<#863 (comment)>
(I think the PR has been based on an older branch, but it has been
quite a while and synchronizing the code becomes not very possible. I
will let Claude to take a look to see how to "rebase" it to be
cleaner, but it may take some time..)
—
Reply to this email directly, view it on GitHub
<#863 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APXLOT4X3IPH3BPISIG2GC34RAZU7AVCNFSM6AAAAACWTUQULOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRZGMYDOMJUG4>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Member
|
That would surely help --- I think an easier way is that you FORK again and apply your changes manually to that new branch (since it is mostly located in a few commits). If you have a coding agent, it should be pretty helpful in cherry-picking your commits and apply it to the newer code. (You can also let the AI write a description on how I can verify and confirm the same parameters and what test cases can I use to cross-check with the RFC) |
As was requested I'm adding the details of finding the numbers/parameters. In case the link will break let me add here less readable but more persistent copies of the same. \ <ar://n5OSUxB5aHM-9j5Sz54qOb3hUgHY9IwqUeA-kwTIvxk> \ `body` of the tx \ ---- I wanted to \\[add hashing for the curve to `arkworks-rs`\\](\n\n<https://github.com/arkworks-rs/algebra/pull/863>\n\n), and found out that it `assert` the correct mapping of the generator from the isogenous curve to the target one, hence I had to find the matching generator on the iso-curve.\n\nTLDR \\\\\\\nthe generator is $55066263022277343669578718895168534326250603453777594175500187360389116729240, 32670510020758816978083085130507043184471273380659243275938904335757337482424$\n\nSearching for it took me nowhere (and now you can come to this page), so that left me with *finding* it mathematically. Thanks a lot to the RFC authors for the reference implementation making this a straightforward task.\n\nThe reference implementation file defining this isogeny is <\n\n<https://github.com/cfrg/draft-irtf-cfrg-hash-to-curve/blob/main/poc/iso_values.sage>\n\n\\>. And below is a patch to `print` and check the values of interest with it. (Use `sage iso_values.sage` to run it.)\n\n```diff\ndiff --git a/poc/iso_values.sage b/poc/iso_values.sage\nindex 4a6606e..59b8c48 100644\n--- a/poc/iso_values.sage\n+++ b/poc/iso_values.sage\n@@ -20,6 +20,7 @@ def show_elm(val):\n \n def show_iso(iso):\n (xm, ym) = iso.rational_maps()\n+ print(xm)\n maps = (xm.numerator(), xm.denominator(), ym.numerator(), ym.denominator())\n strs = ("x\\\\_num", "x\\\\_den", "y\\\\_num", "y\\\\_den")\n mstr = ""\n@@ -66,10 +67,29 @@ def iso_secp256k1():\n A = 0\n B = 7\n E = EllipticCurve(GF(p), [A, B])\n+\n+ G = E(0x79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798, 0x483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8)\n+ print("generator")\n+ print(G)\n+\n Ap = 0x3f8731abdd661adca08a5558f0f5d272e953d363cb6f0e5d405447c01a444533\n Bp = 1771\n Ep = EllipticCurve(GF(p), [Ap, Bp])\n- iso = EllipticCurveIsogeny(E=E, kernel=None, codomain=Ep, degree=3).dual()\n+ \n+ iso_straight = EllipticCurveIsogeny(E=E, kernel=None, codomain=Ep, degree=3)\n+ print("isogenous generator")\n+ # Gp = iso_straight(G)\n+ Gp = Ep(75295888890003590383366995344834012177557063699577440394299653383124903397514, 82553647407850972504999846303729620951309077682374043495922869307182479212755)\n+ print(Gp)\n+ \n+ iso = iso_straight.dual()\n+ print("does it looks good?")\n+ # print(iso(Gp) - 2*G == G)\n+ print(iso(Gp) == G)\n+\n+ # print(".division_points(3)")\n+ # print(Gp.division_points(3))\n+\n if (- iso.rational_maps()[1])(1, 1) > iso.rational_maps()[1](1, 1):\n iso.switch_sign()\n _iso_secp256k1 = iso\n```
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Adds hashing to the <curves/secp256k1>.
closes: none - just a development
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
v0.5will be merged intomasterPendingsection inCHANGELOG.md: feels like a bad idea untilv0.5is merged intomasterFiles changedin the GitHub PR explorer