Test commit count CI test [DONT MERGE]

What does this PR do?
Tests commit count CI test [DONT MERGE]

Codecov Report

Merging #2351 (5231751) into main (d583039) will not change coverage.
The diff coverage is n/a.

:exclamation: Current head 5231751 differs from pull request most recent head e4ec9d0. Consider uploading reports for the commit e4ec9d0 to get more accurate results

@@           Coverage Diff           @@
##             main    #2351   +/-   ##
=======================================
  Coverage   75.06%   75.06%
=======================================
  Files          62       62
  Lines        5498     5498
=======================================
  Hits         4127     4127
  Misses       1371     1371

Help us with your feedback. Take ten seconds to tell us how you rate us.


Lr norm

LR normalize and balance_fields flags cause predictions to changes

just a question and a couple documentation typos.  other than that, tests are passing and code looks good.


MG & MIs

Dear @DeeshaPatel
My MG and MIS is ready for review.
Thank you for your time and valuable feedback.

Dear @MinMah23, I have done my review for your both documents. :)


Remove storing of activities and users in fetch spaces

When fetching a group of spaces, we only need to store the user data from the "to user" in a one-on-one conversation. The recents widget does not need any other data in the redux store, ie: other participants and space activities.
I've also split out fetchSpacesEncrypted from fetchSpaces to make things more readable.

Adam this made rendering much much faster on chrome. I had a question though out of curiosity, how did you find out we were storing unnecessary data and what changes that you made here reflect that?


Group key witness support

Implements changes outlined in https://github.com/lightninglabs/taproot-assets/issues/466.
Known TODOs:

[x] VM execution in DeriveGroupKey to verify witness correctness
[x] Copy group witness to asset.PrevWitnesses where need so the witness gets serialized
[x] update proof.Verify to use new genesis asset definition
[x] Resolve dependency cycle with tapscript package
[ ] New unit test cases for features introduced in past PRs
[ ] New unit test cases for features of this PR

Approach:

Modify the virtualTx methods to add new branches for creating a virtual TX for a genesis grouped asset.

Modify the VM to prevent an early exit for genesis grouped assets, and add early assertions for such assets.

Update DeriveGroupKey to sign over a virtualTx representing minting of a genesis grouped asset.

Update DeriveGroupKey to accept external scripts and support different signing methods.

Open questions:

Can we reuse the existing VirtualTxSigner for the GenesisSigner?

Is there other data needed for a group key script path authorization here besides a single TapLeaf? In CreateTaprootSignature I see validation against other fields but not use of those other fields for signing.

How can we resolve the tapscript dependency cycle? I see two issues, one being the placement of RawKeyGenesisSigner code, which AFAICT is only used in tests. The other is that, the asset package can't call methods from the tapscript package without causing a cycle, but with the current setup DeriveGroupKey must build a virtual TX at some point. I think one option may be to move some of the group signing logic into the tapscript package. As-is I think the definition of DeriveGroupKey could move into tapscript and self-contained parts of that logic could stay in the asset package.

I don't see a neat solution to the dependency cycle issues, so just moving some of the virtual TX creation logic for these special group genesis assets into asset and accepting some duplication with tapscript to keep this moving forward.

What do you mean by this? The script path auth is just the existing witness field right? Or do you mean the group key derivation reveal? The group key witness (for minting) should be more or less the same as the flow for signing virtual txns normally. IIUC, we don't need any new fields in the VInput, instead the existing fields are populated as normal. The main difference now is that we swap out the public key (not the input asset) for the group key asset.

From the RPC level, what data types to I need to provide as arguments when minting a grouped asset into a group, where I want to use a script path and not a simple signature?
And then, how does that get passed in to the final signing descriptor (I think this part is clearer to me now).

From the RPC level, what data types to I need to provide as arguments when minting a grouped asset into a group, where I want to use a script path and not a simple signature?

Ah, I think for that, we'd just make sure we're able to handle such spends internally via unit tests, then defer the expansion of the RPC interface to satisfy more complex group minting conditions. So to start, we can just allow them to specify their own group key and we use that instead of the auto generated one. The caller would then need to handle storing all the tapcript leaves, just like the itests we have to do non keyspend spends for normal assets.
With that said, if we wanted to do some design sketches, I'd say:

what data types to I need to provide as arguments when minting a grouped asset into a group, where I want to use a script path and not a simple signature?

You'd need to specify:

The group key to use.
The full witness field for that group key.

We can't necessarily generate the witness ourselves, as we may not even control the key used. Instead, the caller would use the vPSBT APIs to craft the minting transaction. To facilitate this, we'd need to expose calls that can be used to lock UTXOs (they want to know which outpoint will be used), or we'd add that as an arg to the minting process.

And then, how does that get passed in to the final signing descriptor (I think this part is clearer to me now).

We don't do signing in this context, as we might not be able to. The minting witness is just the witness of a "normal" spend, but it can spend the "all zero" outpoint, as it's a proof that the group key can be satisfied. You can think of it like the PSBT based channel funding for lnd: we don't know what inputs the caller is using, but we can provide them with the information needed to make the funding transaction (the funding outpoint).

Read through the feedback, my current plan is:

Move the virtual TX construction logic behind another interface, asset.GenesisTxBuilder, to resolve dependency issues.
Simplify DeriveGroupKey to only handle the key spend path with an empty tapscript root.
New methods on Asset so its clearer when a grouped genesis asset needs a witness or already has a witness.
Update asset.new() or add logic to the minter s.t. the group key witness is copied into the asset during minting.

Latest version is on this branch with the described changes. The commits were re-done and should be relatively readable.
Moving on to improving test coverage.

Another source of dependency issues:
https://github.com/lightninglabs/taproot-assets/blob/843584a12a7bcefd85003f584c24fe48e8705713/commitment/asset.go#L151
Verifying the group witness on asset commitment insertion.

Verifying the group witness on asset commitment insertion.

So I think we can actually move this out entirely. Looks like the only time we call VerifySignature is Upsert and also when we make a new asset commitment. Now that we have the group key reveal stuff in the proof file verification, we can just move that there. So now we only verify the signature when we create assets, and also when we verify proofs for assets.

Addressed a few comments, updated this main branch and the WIP branch with unfinished test fixes here:
https://github.com/lightninglabs/taproot-assets/tree/group-key-witness-support-wip
There was an issue with not using the correct branching condition in VirtualTx for reaching VirtualGenesisTxIn - since VirtualTx is called both when building the Virtual TX for signing, and when verifying in the VM, I think we'd actually want to branch on the genesis grouped asset both before and after the group witness has been attached.
However, VM validation for normal state transitions is now failing, so I think the branch condition as I've written it is too broad - I think this could be fixed by also considering prevAssets at that point. So something like:
If both the new asset and the prev asset are genesis grouped assets, then it must be a genesis grouped asset mint, branch to VirtualGenesisTxIn.
We still have the sanity check very early in the VM that I think should stay as HasGenesisWitnessForGroup.
The WIP branch also has some fixes for other tests but those haven't been exercised yet since they usually depend on the VM working anyways.

Latest push is further fixes to address some unit test fixes + fix group insertion when syncing from a Universe (h/t guggero).
Most itests pass, but reissuance does not work - adding a GroupVerifier interface now to be used when verifying proofs, to check if there is a verified group that matches the group key in the proof.

Adding a groupVerifier has worked for fixing the reissuance test.
From offline discussion, I realized that the GroupKeyReveal should be omitted for genesis proofs of reissued assets, and instead the asset's group key should only be checked against stored group keys to ensure that the daemon has previously verified the group anchor (along with group witness verification).
However, this introduces new ordering constraints in many places where we handle grouped assets (storing to disk, verifying a proof, or populating a universe tree). The anchor asset for a group MUST be inserted before attempting to verify or store any other member of that asset group.
Right now many DB operations are unordered over e.x. the set of all assets in a minting batch, which I'm updating now.

All itests are now passing locally & with the current state of this PR. Latest changes were to sort assets such that group anchors are inserted on disk before any possible group reissuances, since group key verification now relies on the group anchor being present on disk.
Pending changes are addressing reviewer feedback and changing the GroupKey.TapscriptRoot member from [32]byte to []byte. As an array the nil-ness of the field is harder to test, since it is set to [32]byte{0x0} when not explicitly initialized.

TODOs:

[ ] Adding feedback from previous review
[ ] Array to slice changes

Updated to address reviewer feedback + BIP vectors are updated to add the new proof reveal fields + GroupKey.TapscriptRoot is now []byte, which simplified handling in a few places.

Rebased and addressed most recent feedback.


#224 Does not work Jest snapshot testing

Adds an input check that fixes the bug in #224
Also adds a string type check which is missing in some cases when i use jest

This also fix an issue when the input is blured because of page change and form unmount, thanks :+1:
I've published a fix on npm in the meantime => npm install -S<EMAIL_ADDRESS>

Any chance getting this merge please ?

Any progress on a fix? This is still an issue and I am getting an error with react-test-renderer and Storybook7!
  TypeError: Cannot read properties of null (reading 'addEventListener')

      at node_modules/react-input-mask/lib/react-input-mask.development.js:219:11
...