Skip to content

torchwood: implement spicy signatures and witness policies - #34

Merged
FiloSottile merged 2 commits into
mainfrom
push-kqxupwqqwyrk
Dec 18, 2025
Merged

torchwood: implement spicy signatures and witness policies#34
FiloSottile merged 2 commits into
mainfrom
push-kqxupwqqwyrk

Conversation

@FiloSottile

Copy link
Copy Markdown
Owner

No description provided.

@FiloSottile
FiloSottile force-pushed the push-kqxupwqqwyrk branch 2 times, most recently from be6a777 to ed7fc66 Compare November 12, 2025 01:03
Comment thread spicy.go Outdated
Comment thread spicy.go Outdated
Comment thread spicy.go Outdated
// verify the signed checkpoint included in the proof. If open returns an error,
// it is returned directly. If the proof is valid but does not verify the record
// hash rh at the given index, a *[VerifyRecordError] is returned.
func VerifyProof(origin string, open func([]byte) (*note.Note, error), rh tlog.Hash, proof []byte) error {

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.

The parameter open func([]byte) (*note.Note, error) is a little more indirection than I expected here?

Is the purpose of this basically to encapsulate the Note verification params within the callback rather than having it in the VerifyProof signature?

The tests below satisfy it by providing...

openFunc := func(msg []byte) (*note.Note, error) {
	return note.Open(msg, note.VerifierList(verifier))
}

... but I'm wondering what else one would ever really provide? Is there an example of the kind of variation we want to enable there? (It would make sense to me if we were either optionalizing a dependency, or wrapping potentially complicated and opinionated verification logic, but it doesn't look like it's the former -- note.Verifiers+note.Verifier are already interfaces and boil all the way down to primitives -- and I'm not really sure what the latter would be either.)

The only thing I can think of is to use this indirection to shrug off an UnverifiedNoteError. I guess that's useful (for testing, if nothing else), but I don't know if that feels like a great reason to increase the complexity of using VerifyProof so very significantly.

I might be uncreatively missing some other purpose, though :)

If this does stay as a parameter that's a function type: I'd request the docs explain it a little and give a suggestion of the most common implementation. I'm not sure how readily a new approacher of the library would guess the correct placement of puzzle pieces, otherwise.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

or wrapping potentially complicated and opinionated verification logic

That's what we are doing, this is where the witness policy goes, with its quorums and multi-level groups.

We simply don't have the function to plug in it yet. Once we do we'll point to it in the docs.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The func parameter is now gone, replaced by the Policy interface.

Main thing I am not sure about is whether the Policy should include the log origin, either explicitly as an Origin method (which can be empty for component policies and inherits through composite ones), or implicitly by making a LogVerifier which checks the origin line while verifying the note message.

Comment thread spicy.go Outdated
Comment thread spicy.go Outdated
@FiloSottile
FiloSottile force-pushed the push-kqxupwqqwyrk branch 2 times, most recently from 5a0a3cb to cbed770 Compare December 13, 2025 19:16
@FiloSottile FiloSottile changed the title torchwood: implement c2sp.org/tlog-proof formatting and verification torchwood: implement spicy signatures and witness policies Dec 13, 2025
@FiloSottile
FiloSottile merged commit 0919b5b into main Dec 18, 2025
16 checks passed
@FiloSottile
FiloSottile deleted the push-kqxupwqqwyrk branch December 18, 2025 19:03
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.

2 participants