Skip to content

Update DIPs regarding use of BLS12-381-reversed#25

Merged
thephez merged 7 commits into
masterfrom
codablock-bls-patch
Nov 9, 2018
Merged

Update DIPs regarding use of BLS12-381-reversed#25
thephez merged 7 commits into
masterfrom
codablock-bls-patch

Conversation

@codablock
Copy link
Copy Markdown
Contributor

@codablock codablock commented Oct 26, 2018

See individual commits.

Closes #23

@codablock codablock mentioned this pull request Oct 26, 2018
3 tasks
Comment thread dip-0003.md Outdated
Comment thread dip-0004.md Outdated
| ipAddress | byte[] | 16 | IPv6 address in network byte order. Only IPv4 mapped addresses are allowed (to be extended in the future) |
| port | uint_16 | 2 | Port (network byte order) |
| keyIDOperator | CKeyID | 20 | The operators public key hash |
| pubKeyOperator | CKeyID | 48 | The operators public key |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread dip-0006/bls_signature_scheme.md Outdated
It is necessary to serialize BLS primitives into a byte representation so that they can be used inside other serialized data structures. How this is performed depends on the curves and libraries used. This has not been defined yet and this document will be updated later.
It is necessary to serialize BLS primitives into a byte representation so that they can be used inside other serialized data structures.
The serialization format is described in the Chia [bls-signatures](https://github.com/Chia-Network/bls-signatures/blob/master/SPEC.md)
repository.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason for the line breaks here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's very hard to read/edit otherwise in my editor. In Markdown, single-newlines are ignored when rendered, so it's the same as if there was no newline.

I'd suggest to limit line width in the future when we create DIPs to make editing/reading the plain Markdown easier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have this problem in all DIPs and nearly all paragraphs are in a single line...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see it as a "problem" since editors can be made to soft fold, but, I also understand and think if we do this then let's set at something standard like 72 or 80 and hard-wrap at that then.

Comment thread dip-0003.md

The ProRegTx contains 3 public key IDs, which represent 3 different roles in the masternode and define update and voting rights. A "public key ID" refers to the hash160 of an ECDSA public key. These are:
The ProRegTx contains 2 public key IDs and one BLS public key, which represent 3 different roles in the masternode and define update and voting rights.
A "public key ID" refers to the hash160 of an ECDSA public key. The keys are:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same question about line breaks as below

Comment thread dip-0004.md
@codablock
Copy link
Copy Markdown
Contributor Author

Is this ready for merge? No ACKs yet...

@thephez
Copy link
Copy Markdown
Collaborator

thephez commented Nov 9, 2018

Only question I see is whether we leave the line breaks or remove them. Content looks fine.

Copy link
Copy Markdown
Contributor

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

ACK

I don't have a strong opinion about line breaks, I'm fine with either one.

@codablock
Copy link
Copy Markdown
Contributor Author

IMHO we should leave them and in the future change all paragraphs to be multiline. As explained, it doesn't change how it is rendered but makes live a lot easier when editing these files

@nmarley
Copy link
Copy Markdown
Contributor

nmarley commented Nov 9, 2018

I missed your comment several days ago @codablock. I'm fine with it, just wondered why you left it that way (and now I saw the explanation).

Copy link
Copy Markdown
Contributor

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

ACK

@thephez thephez merged commit 52aee60 into master Nov 9, 2018
@thephez thephez deleted the codablock-bls-patch branch January 8, 2019 17:14
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