Skip to content

Add Clone bound to BitIter in Jet::decode() #343

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.

Sign up for GitHub

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

Open
topologoanatom wants to merge 1 commit into
base: master
Choose a base branch
from
+7 −7
Open

Add Clone bound to BitIter in Jet::decode() #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/bit_encoding/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl<J: Jet> DagLike for (usize, &'_ [DecodeNode<J>]) {
}
}

pub fn decode_expression<'brand, I: Iterator<Item = u8>, J: Jet>(
pub fn decode_expression<'brand, I: Iterator<Item = u8> + Clone, J: Jet>(
Copy link
Collaborator

@apoelstra apoelstra Feb 11, 2026

Choose a reason for hiding this comment

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

In 2db8d12:

This bound is not necessary. Neither are any of the other bounds you added.

Copy link
Author

@topologoanatom topologoanatom Feb 11, 2026

Choose a reason for hiding this comment

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

Could you elaborate on why the bound is not necessary? Can the related issue be solved without adding the bound, or are you referring to bounds other than the one in the Jet definition? If the latter, I added the minimal number of bounds required for the code to compile.

Copy link
Collaborator

@apoelstra apoelstra Feb 11, 2026

Choose a reason for hiding this comment

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

The function worked without the bound -- adding the bound tightens it, but nothing in the function uses the extra bound. Doing this makes the API less useful, not more useful, because it reduces the number of ways you can call these functions.

The PR description claims that it adds a bound to BitIter but it does not.

Copy link
Author

@topologoanatom topologoanatom Feb 12, 2026

Choose a reason for hiding this comment

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

but nothing in the function uses the extra bound.

If you're referring to the function in the second code block of the issue description, that's pseudocode to illustrate the motivation. The actual implementation uses clones.

The PR description claims that it adds a bound to BitIter but it does not.

BitIter already derives Clone, so to obtain cloneable iterators, we need to bound the iterator type parameter. Moreover, adding the bound directly to BitIter itself would require a substantial number of changes (possibly all of them) in this codebase, plus changes to the autogenerated jet::init:: files. Sorry if the description was confusing, but this approach seems like the minimal intervention to achieve the issue's goal.

If the existing BitIter API provides a way to rewind steps that I somehow missed, please point it out. Otherwise, is the issue's motivation worthy of restricting bounds?

ctx: &types::Context<'brand>,
bits: &mut BitIter<I>,
) -> Result<ArcNode<'brand, J>, Error> {
Expand Down Expand Up @@ -237,7 +237,7 @@ pub fn decode_expression<'brand, I: Iterator<Item = u8>, J: Jet>(

/// Decode a single Simplicity node from bits and
/// insert it into a hash map at its index for future reference by ancestor nodes.
fn decode_node<I: Iterator<Item = u8>, J: Jet>(
fn decode_node<I: Iterator<Item = u8> + Clone, J: Jet>(
bits: &mut BitIter<I>,
index: usize,
) -> Result<DecodeNode<J>, Error> {
Expand Down
2 changes: 1 addition & 1 deletion src/jet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub trait Jet:
fn encode<W: Write>(&self, w: &mut BitWriter<W>) -> std::io::Result<usize>;

/// Decode a jet from bits.
fn decode<I: Iterator<Item = u8>>(bits: &mut BitIter<I>) -> Result<Self, decode::Error>;
fn decode<I: Iterator<Item = u8> + Clone>(bits: &mut BitIter<I>) -> Result<Self, decode::Error>;

/// Obtains a C FFI compatible environment for the jet.
fn c_jet_env(env: &Self::Environment) -> &Self::CJetEnvironment;
Expand Down
2 changes: 1 addition & 1 deletion src/node/commit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ impl<J: Jet> CommitNode<J> {
/// or the witness is provided by other means.
///
/// If the serialization contains the witness data, then use [`RedeemNode::decode()`].
pub fn decode<I: Iterator<Item = u8>>(bits: BitIter<I>) -> Result<Arc<Self>, DecodeError> {
pub fn decode<I: Iterator<Item = u8> + Clone>(bits: BitIter<I>) -> Result<Arc<Self>, DecodeError> {
use crate::decode;

// 1. Decode program with out witnesses.
Expand Down
2 changes: 1 addition & 1 deletion src/node/construct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl<'brand, J: Jet> ConstructNode<'brand, J> {
/// or the witness is provided by other means.
///
/// If the serialization contains the witness data, then use [`crate::RedeemNode::decode()`].
pub fn decode<I: Iterator<Item = u8>>(
pub fn decode<I: Iterator<Item = u8> + Clone>(
context: &types::Context<'brand>,
mut bits: BitIter<I>,
) -> Result<Arc<Self>, crate::decode::Error> {
Expand Down
4 changes: 2 additions & 2 deletions src/node/redeem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ impl<J: Jet> RedeemNode<J> {
mut witness: BitIter<I2>,
) -> Result<Arc<Self>, DecodeError>
where
I1: Iterator<Item = u8>,
I2: Iterator<Item = u8>,
I1: Iterator<Item = u8> + Clone,
I2: Iterator<Item = u8> + Clone,
{
// 0. Set up a type to help with the call to `convert` below
struct DecodeFinalizer<'bits, J: Jet, I: Iterator<Item = u8>> {
Expand Down
Loading