Simplify ZSA implementation

[ConstanceBeguier]

• Update comments
• Reduce diff with upstream
• Unify Rust version to 1.71 (Cargo.toml, README.md and rust-toolchain.toml)
• Remove pinned version for half and font-kit in Cargo.toml
• Remove public unsplittable function
• Clean tests/zsa.rs by creating one Merkle tree with all notes and by generating new keys to avoid using the same address for both sender and recipient in bundles
• Simplify IssueAction::verify(...) by moving the AssetBaseCannotBeIdentityPoint check outside the loop.
  • Rationale: Previously we checked that for each note, its asset is not equal to identity point and for each note, its asset is equal to the issue asset. Now we checked that the issue asset is not equal to the identity point and for each note, its asset is equal to the issue asset. Those two versions will fail in the same cases.
  • We removed action_verify_invalid_for_asset_base_as_identity, issue_bundle_cannot_be_signed_with_asset_base_identity_point and issue_bundle_verify_fail_asset_base_identity_point tests because it is now very difficult to create a test for this case. To create a test for this case, we have to choose an ik and an asset_desc_hash such that the AssetBase derived from them is equal to the identity point.

[PaulLaux]

Great improvment!
added minor comments.
also

fn verify_unique_spent_nullifiers(bundle: &Bundle<Authorized, i64, OrchardZSA>) -> bool {
        let mut seen = HashSet::new();
        bundle
            .actions()
            .iter()
            .all(|action| seen.insert(action.nullifier().to_bytes()))
}

to simplify

[PaulLaux]

consider

if hm.len() == 1 {
        if let Some((spends, outputs)) = hm.get_mut(&AssetBase::native()) {
            let pad_spends = MIN_ACTIONS.saturating_sub(spends.len());
            let pad_outputs = MIN_ACTIONS.saturating_sub(outputs.len());

            spends.extend(iter::repeat_with(|| (SpendInfo::dummy(AssetBase::native(), rng), None)).take(pad_spends));
            outputs.extend(iter::repeat_with(|| (OutputInfo::dummy(rng, AssetBase::native()), None)).take(pad_outputs));
        }
    }

[PaulLaux]

or even more general

    // If native asset exists, pad spends and outputs to meet the minimum action requirements.
    if let Some((spends, outputs)) = hm.get_mut(&AssetBase::native()) {
            let pad_spends = MIN_ACTIONS.saturating_sub(spends.len());
            let pad_outputs = MIN_ACTIONS.saturating_sub(outputs.len());

            spends.extend(iter::repeat_with(|| (SpendInfo::dummy(AssetBase::native(), rng), None)).take(pad_spends));
            outputs.extend(iter::repeat_with(|| (OutputInfo::dummy(rng, AssetBase::native()), None)).take(pad_outputs));
    }

it is 1 only if MIN_ACTIONS is 2. this one captures bigger values for MIN_ACTIONS

[ConstanceBeguier]

I kept the first suggested version

[PaulLaux]

will it work for MIN_ACTIONS > 2 (the entire function)?

[ConstanceBeguier]

As discussed offline, it will work when MIN_ACTIONS > 2.
I added some comments to clarify the meaning of the two if

[PaulLaux]

ideally we have one test that gets this error. in this version we have none.

[ConstanceBeguier]

As explained in the last item of the PR description, it is now impossible to test this error.
To test this error, we have to choose an ik and an asset_desc_hash such that the AssetBase derived from them is equal to the identity point.

[ConstanceBeguier]

The drawback of the modification in the verify function is the fact that it is no longer possible to test AssetBaseCannotBeIdentityPoint error (see previous comment).
The advantage is the fact that the verify function (line 167) is now simpler and faster.

[PaulLaux]

I see it now. agreed.

[PaulLaux]

for let (mut awaiting_nullifier_bundle, _) = IssueBundle::new( line 156
mut not needed.

[ConstanceBeguier]

The mut is required for the add_recipient inside the assert! line 166.

[ConstanceBeguier]

Another option is to remove the mut line 156 and clone awaiting_nullifier_bundle inside the assert line 166.

[PaulLaux]

i see. both are acceptable.

[PaulLaux]

Approved pending #178 (comment)