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

rebased old BLS rework. For discussion purposes. #1233

Closed
wants to merge 4 commits into from

Conversation

protolambda
Copy link
Contributor

Honestly, the BLS interface as-is is ok*. But if we still want to separate out application layer from the inner BLS layer, we may want to consider something alike to this PR; a rebased version of the initial work by @JustinDrake

I propose we discuss if we move forward with it, making it a post spec-freeze non-breaking change, i.e. fully backwards compatible for signatures during testnet. And keep this apart from the BLS spec-update itself, as a presentation thing.

*: ok, test-vectors will adopt the spec status to alleviate the typing inconsistency highlighted in #964

```python

def bls_verify(pubkey: BLSPubkey, self_signed_object: Container, domain: BLSDomain) -> bool:
return bls.bls_verify(pubkey, domain + signed_root(self_signed_object), self_signed_object.signature)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we go this way (which I'm really not sold on!! I think there's a lot of value in reducing the depth of the abstraction tower that readers have to waddle through, and the verify(pub, msg, sig) format has a lot going for it given it's simple and familiar, and also symmetry with bls_verify_multiple is valuable), can we at least call it bls_verify_signed_container? Would make it clearer to readers what's going on imo.

@JustinDrake
Copy link
Contributor

I suggest we stick with current interface and go ahead with the misc cleanups (e.g. add BLSDomain type).

@JustinDrake JustinDrake added the milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet label Jun 30, 2019
@protolambda
Copy link
Contributor Author

@JustinDrake You can go ahead with the misc. cleanups in the helper cleanup PR. As expressed in the closed issue, I generally think the primary part of this PR should not be considered for spec-freeze. So also dislike labeling this as "spec-freeze", for just a BLSDomain alias.

Instead I want to keep the discussion alive to some time after freeze, where there is less time pressure, and we can find some sweet spot (not necessarily with self-signed objects in interface, but possible) in separation of BLS and spec.

@protolambda
Copy link
Contributor Author

Unlabeling "spec freeze", in favor of #1246 which implements Domain and DomainType, but leaves interface as is. A simple and clean solution for freeze. We can experiment more with this PR later, or not, see what the cons/pros are here.

@protolambda protolambda removed the milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet label Jun 30, 2019
@JustinDrake
Copy link
Contributor

I'd say close this PR and revisit when we merge in the BLS standardisation post-freeze.

@protolambda protolambda deleted the bls-usage-update branch February 9, 2020 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:discussion general:presentation Presentation (as opposed to content)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants