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

bip-325: correct placement of challenge #1005

Merged
merged 1 commit into from
Oct 8, 2020

Conversation

kallewoof
Copy link
Contributor

In #1003 (comment) (where I adopt the approach here), it is pointed out that the message signature going into the scriptPubKey of the spending transaction is weird.

It should go into the scriptSig and/or scriptWitness, and the scriptPubKey for the spending tx is OP_RETURN.

@kallewoof
Copy link
Contributor Author

Ping @ajtowns.

Copy link
Contributor

@ajtowns ajtowns left a comment

Choose a reason for hiding this comment

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

Hmm, I guess that was either a copy and paste error, or leftover from the previous approach?

@@ -60,8 +60,10 @@ The "to_sign" transaction is:
vin[0].prevout.hash = to_spend.txid
vin[0].prevout.n = 0
vin[0].nSequence = 0
vin[0].sigScript = [ signet_challenge first data push, if any ]
vin[0].scriptWitness = [ signet_challenge second data push, if any ]
Copy link
Contributor

Choose a reason for hiding this comment

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

These are from the signet solution, not the signet challenge, and there's only a single data push of the serialization of the scriptSig and and scriptWitness concatenated.

Copy link
Contributor Author

@kallewoof kallewoof Oct 6, 2020

Choose a reason for hiding this comment

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

Doh, I meant the signet solution.

There's a single push, of the 1-2 pushes of sigScript followed by optional scriptWitness, right? That's what I'm referring to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gah, I meant the solution, not challenge.

@kallewoof kallewoof force-pushed the 202010-signet-sigscript branch from ccbb7bf to fd50eea Compare October 6, 2020 01:44
@@ -60,8 +60,10 @@ The "to_sign" transaction is:
vin[0].prevout.hash = to_spend.txid
vin[0].prevout.n = 0
vin[0].nSequence = 0
vin[0].sigScript = [ signet_solution first data push, if any ]
vin[0].scriptWitness = [ signet_solution second data push, if any ]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
vin[0].scriptWitness = [ signet_solution second data push, if any ]
vin[0].scriptWitness = [ signet_solution scriptWitness (y bytes), if any ]

Maybe refer to it with the length to avoid the "data push" confusion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea

@kallewoof kallewoof force-pushed the 202010-signet-sigscript branch from fd50eea to bb989a6 Compare October 6, 2020 06:34
@ajtowns
Copy link
Contributor

ajtowns commented Oct 7, 2020

Looks good to me

@kallewoof
Copy link
Contributor Author

Ping @luke-jr

@luke-jr luke-jr merged commit dcc6a6a into bitcoin:master Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants