Add proof type to universe identifer #540

ffranr · 2023-10-03T17:30:49Z

Towards the end result of two separate universe trees for each asset/asset group. One tree for issuance proofs and one for transfer proofs.

universe/interface.go

Roasbeef · 2023-10-04T00:27:02Z

universe/interface.go

@@ -92,6 +92,27 @@ func (i *Identifier) StringForLog() string {
 		i.MsSmtNamespace(), i.AssetID[:], groupKey, i.ProofType)
 }

+// ValidateProofUniverseType validates that the proof type matches the universe
+// identifier proof type.
+func ValidateProofUniverseType(proof proof.Proof, uniID Identifier) error {


Should add some unit + itests for this.

I've added a unit test for this. ValidateProofUniverseType is already covered in itests for the positive case. Should I add more on the itest front for this PR?

guggero

Did a quick first pass, changes make sense, nice work!

universe/interface.go

rpcserver.go

universe/interface.go

universe/base.go

Roasbeef · 2023-10-07T00:25:38Z

tapdb/sqlc/migrations/000007_universe.up.sql

+
+    -- This field is an enum representing the proof type stored in the given
+    -- universe.
+    proof_type TEXT NOT NULL CHECK(proof_type IN ('issuance', 'transfer'))


Also IIUC, namespace_root now also includes this as a prefix. May want to update some of the comments in the schema file.

Right now, the schema doesn't explain what's in namespace_root:

-- For the namespace root, we set the foreign key constraint evaluation to -- be deferred until after the database transaction ends. Otherwise, if the -- root of the SMT is deleted temporarily before inserting a new root, then -- this constraint is violated as there's no longer a root that this -- universe tree can point to. namespace_root VARCHAR UNIQUE NOT NULL REFERENCES mssmt_roots(namespace) DEFERRABLE INITIALLY DEFERRED,

Roasbeef · 2023-10-07T00:32:16Z

rpcserver.go

+		// The target proof type was unspecified. Assume that the proof
+		// type is a transfer proof (more transfer proofs exist then
+		// issuance proofs).
+		universeID.ProofType = universe.ProofTypeTransfer


Hmm, maybe we should just return an error in this case? Otherwise if they want issuance, then we error out?

The caller (client calling QueryProof) may not know if the proof they're querying for is a transfer or issuance proof. The universe RPC proof courier is one such caller. And the caller shouldn't need to specify the proof type. Proof type specification only helps the server figure out where the proof is stored. In other words, the client already provides the necessary args for proof identification.

Roasbeef · 2023-10-07T00:32:54Z

rpcserver.go

+
+		// The proof type was originally unspecified by the client. We
+		// will therefore try again with the other proof type.
+		switch universeID.ProofType {


Ah ok, I see the switcheroo here.

I do wonder if we should just hide this one layer deeper, so at the layer right above the db where methods like FetchIssuanceProof are implemented.

itest/assertions.go

rpcserver.go

Roasbeef · 2023-10-07T00:36:59Z

itest/universe_test.go

+		// Construct universe namespace.
+		proofType, err := tap.UnmarshalUniProofType(newRoot.Id.ProofType)
+		require.NoError(t.t, err)
+		uniNamespace := fmt.Sprintf("%s-%s", proofType, uniKey)


Hmm, still thinking about if we want to also manifest the prefix on this level, which arguably could/should just be something internal.

I think will get a better feel of how it all looks on the CLI.

ffranr · 2023-10-11T00:25:55Z

@Roasbeef @guggero There are universe events stats changes that we need following the addition of the proof type to the universe identifier. I'll open a new PR (based on this PR) to address those changes.

I would prefer not to block this PR on those changes so that we can merge this PR and #537

EDIT: PR to address universe event stats changes: #561

ffranr · 2023-10-11T01:19:45Z

This needs a fixup rebase before it's merged.

guggero

Needs a rebase as mentioned. Other than that only two (very optional/non-blocking) nits, so LGTM 💯

rpcserver.go

asset/asset.go

This method returns true if the asset is a genesis asset.

…ootNodes

Roasbeef

LGTM 🎎

ffranr force-pushed the uni-namespace-proof-type branch 2 times, most recently from e00273b to f8eb8f7 Compare October 3, 2023 18:07

Roasbeef linked an issue Oct 3, 2023 that may be closed by this pull request

universe: switch to 128-bit limbs for multiverse leaves or switch to top-level counter sum #479

Open

2 tasks

Roasbeef requested changes Oct 4, 2023

View reviewed changes

guggero self-requested a review October 4, 2023 07:57

guggero reviewed Oct 4, 2023

View reviewed changes

universe/interface.go Outdated Show resolved Hide resolved

universe/interface.go Outdated Show resolved Hide resolved

universe/interface.go Outdated Show resolved Hide resolved

rpcserver.go Show resolved Hide resolved

ffranr force-pushed the uni-namespace-proof-type branch 2 times, most recently from 7b813c1 to 5557ce6 Compare October 5, 2023 15:52

ffranr self-assigned this Oct 6, 2023

ffranr force-pushed the uni-namespace-proof-type branch 2 times, most recently from e224f9d to 9478962 Compare October 6, 2023 19:53

Roasbeef requested changes Oct 7, 2023

View reviewed changes

ffranr force-pushed the uni-namespace-proof-type branch from 9478962 to 641fd47 Compare October 9, 2023 16:24

ffranr requested review from guggero and Roasbeef October 9, 2023 16:24

ffranr mentioned this pull request Oct 9, 2023

Add federation sync configuration SQL table and RPC endpoints #537

Merged

ffranr force-pushed the uni-namespace-proof-type branch 3 times, most recently from b1c6588 to 3ab1c01 Compare October 9, 2023 21:32

ffranr mentioned this pull request Oct 11, 2023

Universe events stats handle universe identity proof type #561

Merged

guggero approved these changes Oct 11, 2023

View reviewed changes

rpcserver.go Outdated Show resolved Hide resolved

asset/asset.go Show resolved Hide resolved

ffranr force-pushed the uni-namespace-proof-type branch from 28e4a6b to dbafc97 Compare October 11, 2023 12:35

ffranr mentioned this pull request Oct 11, 2023

Federation sync config #563

Merged

ffranr added 5 commits October 11, 2023 14:18

universe: add universe proof type enum

001d789

universe: include ProofType in the universe identifier

942d074

rpc: add ProofType to RPC universe ID

f7e3306

rpc: add marshal/unmarshal functions for ProofType RPC type

0a560db

asset: add method Asset.IsGenesisAsset

31e37c5

This method returns true if the asset is a genesis asset.

ffranr added 13 commits October 11, 2023 14:30

rpc+universe: set unspecified universe ID proof type from asset proof

88094ab

rpc+universe: ensure inserted proof type matches universe proof type

4b02687

universe: add unit tests for ValidateProofUniverseType

a44f987

universe: check issuance and transfer universes for previous proof

789fa5a

universe: validate proof type when batch inserting proof leaves

b1a5353

tapdb: add proof type to universe_roots table and queries

b18f473

universe+tapdb: set universe ID proof type when querying via RootNodes

8c260be

tapdb: set default universe ID proof type when generating rand uni ID

d57856d

rpc: add issuance uni and transfer uni roots to QueryAssetRoots

ec8a51f

universe-diff: RootNode selectively unmarshals QueryAssetRoots response

b9935e5

itest: add basic RPC form sanity check in AssertUniverseRootEqual

841fc5e

rpcserver: DeleteAssetRoot takes account of universe proof type

69ce70d

itest: account for new uni ID proof type field when syncing

b2d27d6

ffranr force-pushed the uni-namespace-proof-type branch from 6bb6a33 to b2d27d6 Compare October 11, 2023 13:42

fixup! universe+tapdb: set universe ID proof type when querying via R…

911e2b1

…ootNodes

Roasbeef approved these changes Oct 11, 2023

View reviewed changes

Roasbeef merged commit 951c80e into main Oct 11, 2023
14 checks passed

guggero deleted the uni-namespace-proof-type branch October 11, 2023 21:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add proof type to universe identifer #540

Add proof type to universe identifer #540

ffranr commented Oct 3, 2023

Roasbeef Oct 4, 2023

ffranr Oct 9, 2023

guggero left a comment

Roasbeef Oct 7, 2023

ffranr Oct 9, 2023

Roasbeef Oct 7, 2023

ffranr Oct 9, 2023

Roasbeef Oct 7, 2023

Roasbeef Oct 7, 2023

ffranr commented Oct 11, 2023 •

edited

ffranr commented Oct 11, 2023

guggero left a comment

Roasbeef left a comment

Add proof type to universe identifer #540

Add proof type to universe identifer #540

Conversation

ffranr commented Oct 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffranr commented Oct 11, 2023 • edited

ffranr commented Oct 11, 2023

guggero left a comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

ffranr commented Oct 11, 2023 •

edited