consensus_encoding: Improve macro defined constructor
What the developer wrote
Authored by Tobin C. Harding
consensus_encoding: Improve macro defined constructor
The constructor created by the `encoder_newtype` macro is currently
using the passed in visability modifier. In released code it was
`pub`.
So as not to violate the new type pattern we should not add a public
constructor that ties an encoder to the inner type. This is
functionally the same as making the inner field public.
The constructor is however useful because it saves one having to
explicitly construct the `PhantomData` object. Since construction of
an encoder can be done by calling `encoder` there isn't typically a
need for a public constructor.
Make the constructor defined by `encoder_newtype!` use visability
`pub(crate)`.
This is an API breaking change that bubbles up through all types that
implement `Encodable`.
While we are at it make the function `const` and throw a full stop on
the, admittedly private, docs.
What changed, and why it matters
This commit narrows the visibility of a Rust macro-generated constructor from public to internal-only (pub(crate)). It is described by the author as an API-hardening change to avoid exposing a constructor that would effectively let callers bypass the 'newtype' wrapper pattern. There is no direct evidence in the commit of an exploitable security vulnerability; it is a defensive design improvement.
No immediate security response required. Treat as a normal API-breaking hardening change. Downstream users relying on the public `new()` constructor of encoder newtypes will need to migrate; they can typically construct encoders via the `encoder` method instead.
Security signals we found
Defensive API-hardening: reduces public API surface to prevent accidental or intentional bypass of newtype abstraction
Newtype pattern integrity: public constructor tied to inner encoder could expose inner type semantics
No memory-safety, cryptographic, or consensus bug visible in diff
Evidence from the diff
The encoder_newtype! macro in consensus_encoding/src/encode/mod.rs previously emitted a constructor new() with whatever visibility ($vis) the calling type used, which in released code was pub. The patch fixes the constructor to pub(crate) const fn new(...). The commit message explicitly frames this as preventing violation of the newtype pattern: a public constructor taking the inner encoder would be functionally equivalent to making the inner field public. The change is also made const and a punctuation fix is applied to the doc comment.
Changed components
consensus_encoding/src/encode/mod.rsAll types implementing `Encodable` that use the `encoder_newtype!` macro (API-breaking change)Inspect captured patch +2 / −2
diff --git a/consensus_encoding/src/encode/mod.rs b/consensus_encoding/src/encode/mod.rs
index 05f625cb..ccd5606a 100644
--- a/consensus_encoding/src/encode/mod.rs
+++ b/consensus_encoding/src/encode/mod.rs
@@ -80,8 +80,8 @@ macro_rules! encoder_newtype{
$vis struct $name<$lt>($encoder, core::marker::PhantomData<&$lt $encoder>);
impl<$lt> $name<$lt> {
- /// Construct a new instance of the newtype encoder
- $vis fn new(encoder: $encoder) -> $name<$lt> {
+ /// Construct a new instance of the newtype encoder.
+ pub(crate) const fn new(encoder: $encoder) -> $name<$lt> {
$name(encoder, core::marker::PhantomData)
}
}
Why this scored 22/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.