Skip to content

Conversation

@jimpo
Copy link
Contributor

@jimpo jimpo commented Aug 16, 2017

I was confused about what "data carrier" meant, so I wanted to comment the fAcceptDatacarrier and nMaxDatacarrierBytes fields specifically. Then I figured I'd add docs for the rest of the functions.

@fanquake fanquake added the Docs label Aug 16, 2017
Copy link
Contributor

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Thanks for this!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you lost the stuff noting what vSolutionsRet is when you moved the comment from standard.cpp.

Copy link
Contributor

Choose a reason for hiding this comment

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

(specifically that vSolutionsRet is a vector of the public keys or hashes from the scriptPubKey)

Copy link
Contributor

Choose a reason for hiding this comment

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

(per transaction)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if that's necessary to add here. This is the byte limit for OP_RETURN scripts, then IsStandardTx further restricts the number of OP_RETURN outputs per tx to 1. If this comment were to say per transaction it sort of implies that the data can be split across multiple OP_RETURN outputs as long as the limit is not exceeded.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, missed that it was applied per-scriptPubKey and had an independant per-tx limit, yea, fine as-is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe note that this only works on some standard scriptPubKey types (eg currently does not work on segwit types, nor null_data, for obvious reasons).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to add if you prefer, but that seems like something that the implementation might change. Is it possible that a new CTxDestination for witness programs will be added? It's also interesting that this function does not return a CNoDestination for TX_NULL_DATA scripts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but we can also update the comment when the implementation changes to support more :)

@jimpo jimpo force-pushed the standard-comments branch from 85dc710 to b755f91 Compare August 17, 2017 19:00
@jimpo jimpo force-pushed the standard-comments branch from b755f91 to 360b464 Compare August 18, 2017 21:45
Copy link
Contributor

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

ACK 360b464

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, missed that it was applied per-scriptPubKey and had an independant per-tx limit, yea, fine as-is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but we can also update the comment when the implementation changes to support more :)

@laanwj
Copy link
Member

laanwj commented Aug 22, 2017

ACK 360b464, thanks for adding documentation

@laanwj laanwj merged commit 360b464 into bitcoin:master Aug 22, 2017
laanwj added a commit that referenced this pull request Aug 22, 2017
…d.h.

360b464 Comments: More comments on functions/globals in standard.h. (Jim Posen)

Pull request description:

  I was confused about what "data carrier" meant, so I wanted to comment the `fAcceptDatacarrier` and `nMaxDatacarrierBytes` fields specifically. Then I figured I'd add docs for the rest of the functions.

Tree-SHA512: e6d0cfe6f4a2ab52ae76f984b1f5d8de371ae938e7832be8b02517d868f1caea62fec8888c917a2bd3d8ef74025de7f00dc96923fa56436dc6b190626652bf29
@jimpo jimpo deleted the standard-comments branch September 1, 2017 00:09
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 9, 2019
…standard.h.

360b464 Comments: More comments on functions/globals in standard.h. (Jim Posen)

Pull request description:

  I was confused about what "data carrier" meant, so I wanted to comment the `fAcceptDatacarrier` and `nMaxDatacarrierBytes` fields specifically. Then I figured I'd add docs for the rest of the functions.

Tree-SHA512: e6d0cfe6f4a2ab52ae76f984b1f5d8de371ae938e7832be8b02517d868f1caea62fec8888c917a2bd3d8ef74025de7f00dc96923fa56436dc6b190626652bf29
zkbot added a commit to zcash/zcash that referenced this pull request Dec 17, 2019
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
…standard.h.

360b464 Comments: More comments on functions/globals in standard.h. (Jim Posen)

Pull request description:

  I was confused about what "data carrier" meant, so I wanted to comment the `fAcceptDatacarrier` and `nMaxDatacarrierBytes` fields specifically. Then I figured I'd add docs for the rest of the functions.

Tree-SHA512: e6d0cfe6f4a2ab52ae76f984b1f5d8de371ae938e7832be8b02517d868f1caea62fec8888c917a2bd3d8ef74025de7f00dc96923fa56436dc6b190626652bf29

remove witness

Signed-off-by: Pasta <[email protected]>
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 26, 2021
…standard.h.

360b464 Comments: More comments on functions/globals in standard.h. (Jim Posen)

Pull request description:

  I was confused about what "data carrier" meant, so I wanted to comment the `fAcceptDatacarrier` and `nMaxDatacarrierBytes` fields specifically. Then I figured I'd add docs for the rest of the functions.

Tree-SHA512: e6d0cfe6f4a2ab52ae76f984b1f5d8de371ae938e7832be8b02517d868f1caea62fec8888c917a2bd3d8ef74025de7f00dc96923fa56436dc6b190626652bf29

remove witness

Signed-off-by: Pasta <[email protected]>
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants