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

Consider padding to 1 Sapling spend in non-coinbase bundles #122

Open
str4d opened this issue Feb 12, 2024 · 0 comments
Open

Consider padding to 1 Sapling spend in non-coinbase bundles #122

str4d opened this issue Feb 12, 2024 · 0 comments

Comments

@str4d
Copy link
Contributor

str4d commented Feb 12, 2024

BundleType::Transactional { bundle_required } => {
Ok(if *bundle_required || requested_spends > 0 {
core::cmp::max(requested_spends, 1)
} else {
0
})
}

In the old builder code in zcash_primitives, we would never pad spends. The new logic extends this with a bundle_required case, where the caller can force a 1-spend 2-output bundle to be created even if no spends or outputs are added to the builder.

There is an edge case here that is perhaps non-intuitive: if the caller sets bundle_required = false and then only adds outputs, the resulting bundle will not have any spends (whereas if they instead add spends but no outputs, padding outputs are added). This technically follows the prior logic, but now that we have a separate coinbase mode for the "no spends, no output padding" use case, we might want to reconsider whether we should always pad to at least one spend if we have any outputs.

This is distinct from #121; that PR fixes a regression relative to previous behaviour, whereas this would be a change to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants