Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
signature: replace `signature_derive` with blanket impls by tarcieri · Pull Request #1827 · RustCrypto/traits · GitHub
Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' signature: replace `signature_derive` with blanket impls by tarcieri · Pull Request #1827 · RustCrypto/traits · GitHub
Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' signature: replace `signature_derive` with blanket impls by tarcieri · Pull Request #1827 · RustCrypto/traits · GitHub
Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' signature: replace `signature_derive` with blanket impls by tarcieri · Pull Request #1827 · RustCrypto/traits · GitHub
Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' signature: replace `signature_derive` with blanket impls by tarcieri · Pull Request #1827 · RustCrypto/traits · GitHub
Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); signature: replace `signature_derive` with blanket impls by tarcieri · Pull Request #1827 · RustCrypto/traits · GitHub
Skip to content

signature: replace signature_derive with blanket impls - #1827

Merged
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl
Apr 21, 2025
Merged

signature: replace signature_derive with blanket impls#1827
tarcieri merged 2 commits into
masterfrom
signature/blanket-signature-for-digestsignature-impl

Conversation

@tarcieri

Copy link
Copy Markdown
Member

Adds blanket impls of:

  • Signer for types which impl PrehashSignature + DigestSignature
  • Verifier for types which impl PrehashSignature + DigestVerifier

These blanket impls eliminate the need to have a signature_derive crate, which only existed to write the Signer and Verifier impls for this particular case.

Since this is a breaking change, bumps the version to 3.0.0-pre

@tarcieri
tarcieri requested review from baloo and newpavlovApril 20, 2025 21:08
Adds blanket impls of:
- `Signer` for types which impl `PrehashSignature + DigestSignature`
- `Verifier` for types which impl `PrehashSignature + DigestVerifier`
These blanket impls eliminate the need to have a `signature_derive`
crate, which only existed to write the `Signer` and `Verifier` impls for
this particular case.
Since this is a breaking change, bumps the version to `3.0.0-pre`
@tarcieri
tarcieriforce-pushed the signature/blanket-signature-for-digestsignature-impl branch from 0395a6b to 73f2019CompareApril 20, 2025 21:09
@tarcieri

Copy link
Copy Markdown
MemberAuthor

I think we're at the point where we can do a major version bump, and getting rid of signature_derive would be a great reason to do so, IMO.

IIRC this change was originally suggested by @baloo but I can't find where.

We'll need to verify it works against https://github.com/RustCrypto/signatures

@baloo

Copy link
Copy Markdown
Member

I think that was for the AsyncSigner? (#1419 (comment))

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Aah, right!

@tarcieri

tarcieri commented Apr 21, 2025

Copy link
Copy Markdown
MemberAuthor

@baloo hmm, unfortunately it isn't possible to add that blanket impl because it conflicts with the blanket impl of AsyncSigner for Signer

Getting back to my original complaint on #1419, that means that the sync and async impls will be inconsistent, which is at the very least somewhat annoying

@baloo

baloo commented Apr 21, 2025

Copy link
Copy Markdown
Member

I think that works (see RustCrypto/signatures#945), there are a couple feature flags (downstream) to tweak but otherwise that looks good.

@tarcieri

Copy link
Copy Markdown
MemberAuthor

Okay, let’s go with it despite the inconsistency. I guess we could add macro_rules to do the other delegations which is simpler than custom derive

@tarcieri
tarcieri merged commit bf47748 into masterApr 21, 2025
@tarcieri
tarcieri deleted the signature/blanket-signature-for-digestsignature-impl branch April 21, 2025 21:49
tarcieri added a commit that referenced this pull request May 1, 2025
…)"
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
tarcieri added a commit that referenced this pull request May 1, 2025
…)" (#1840)
This reverts commit bf47748.
Per #1831 this change breaks inference when there is a single explicit
impl of the `Signer` trait.
It also wasn't possible to add corresponding blanket impls to the
`Async*` traits, e.g. `AsyncSigner` for `AsyncDigestSigner`, because of
the existing blanket impl of `AsyncSigner` for `Signer` which we
definitely want to preserve.
As a general rule of thumb, blanket impls only make sense if they work
100% of the time, which doesn't seem to be happening here.
Closes#1831
Sign up for freeto 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.

3 participants

@tarcieri@baloo@newpavlov