feat(block-producer): nullifier map type safety #406
Conversation
Use `Option<NonZeroU32>` instead of relying on zero to indicate none.
There was a problem hiding this comment.
Looks good! Thank you! Let's update the changelog and merge.
Shower thought: is there utility in allowing a nullifier set to be included in the genesis block? Maybe for bootstrapping test networks..
Hmmm - I haven't thought about it much but doing something like that might be tricky. The main reason is that when we build the nullifier SMT the default value for missing nullifiers is 0. This works because we assume that no nullifier can exist in the genesis block (block 0). But if in fact some nullifiers can exist in the genesis block, we'll either need to use a different default value for missing nullifiers, or do something else to distinguish missing nullifiers from the ones that were created in the genesis block.
Done. I don't think I'm seeing the correct merge option - I assume we're doing fast-foward merges? However, I'm only seeing merge with merge commit or squash + merge. |
In most cases, we use the "squash and merge" option. But this is also something to be discussed in the styles and processes discussion I mentioned earlier today. |
|
Oh - another thing, we usually create branches directly in the repos (rather than in forked repos). |
The |
Yep - that's the flow for external contributors. But for the core team, we can create branches in the repos directly. |
Improve the type safety of
TransactionInputs::nullifiersmapping by usingOption<NonZeroU32>instead of relying on zero to indicate none.Shower thought: is there utility in allowing a nullifier set to be included in the genesis block? Maybe for bootstrapping test networks..
(I'm happy to also just drop this PR, its not a major thing)