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

Extract "component" crates into a separate repository #768

Closed
str4d opened this issue Jan 30, 2023 · 9 comments
Closed

Extract "component" crates into a separate repository #768

str4d opened this issue Jan 30, 2023 · 9 comments

Comments

@str4d
Copy link
Contributor

str4d commented Jan 30, 2023

Due to the orchard crate living in a separate repository, we currently have an annoying cyclic dependency between the two repositories, due to zcash_primitives depending on orchard, which depends on zcash_note_encryption. We also intend to move the Sapling protocol logic out into its own repository again (#738).

To improve development interactions between the crates, we should extract the "component crates" into a separate repository and Cargo workspace. By "component crates" I mean "crates that don't depend on the protocols", but this could also be the narrower "crates that the protocols depend on".

@str4d
Copy link
Contributor Author

str4d commented Jan 30, 2023

Here is the current dependency graph between the Rust crates:

graph TD
	n150[equihash]
	n151[f4jumble]
	n230[orchard]
	n232[zcash_note_encryption]
	n336[zcash_address]
	n337[zcash_encoding]
	n338[zcash_client_backend]
	n339[zcash_primitives]
	n340[zcash_proofs]
	n341[zcash_client_sqlite]

	n341 --> n338
	n341 --> n339
	n338 --> n230
	n338 --> n336
	n338 --> n337
	n338 --> n232
	n338 --> n339
	n338 --> n340
	n340 --> n339
	n339 --> n150
	n339 --> n230
	n339 --> n336
	n339 --> n337
	n339 --> n232
	n230 --> n232
	n336 --> n151
	n336 --> n337
Loading

If we define "component crates" as "crates that don't depend on the protocols" then we get this split:

graph TD
	subgraph components
	n150[equihash]
	n151[f4jumble]
	n232[zcash_note_encryption]
	n337[zcash_encoding]
	end

	subgraph protocols
	n230[orchard]
	end

	subgraph main
	n336[zcash_address]
	n338[zcash_client_backend]
	n339[zcash_primitives]
	n340[zcash_proofs]
	n341[zcash_client_sqlite]
	end

	n341 --> n338
	n341 --> n339
	n338 --> n230
	n338 --> n336
	n338 --> n337
	n338 --> n232
	n338 --> n339
	n338 --> n340
	n340 --> n339
	n339 --> n150
	n339 --> n230
	n339 --> n336
	n339 --> n337
	n339 --> n232
	n230 --> n232
	n336 --> n151
	n336 --> n337
Loading

Note that zcash_address is not defined here as a "component crate", because while it doesn't have a dependency on orchard (currently to enable other users to bring their own parsed types), it is internally aware of the existence of the Sapling and Orchard protocols.

If we define "component crates" as "crates that the protocols depend on" then we get this split:

graph TD
	subgraph components
	n232[zcash_note_encryption]
	end

	subgraph protocols
	n230[orchard]
	end

	subgraph main
	n150[equihash]
	n151[f4jumble]
	n337[zcash_encoding]
	n336[zcash_address]
	n338[zcash_client_backend]
	n339[zcash_primitives]
	n340[zcash_proofs]
	n341[zcash_client_sqlite]
	end

	n341 --> n338
	n341 --> n339
	n338 --> n230
	n338 --> n336
	n338 --> n337
	n338 --> n232
	n338 --> n339
	n338 --> n340
	n340 --> n339
	n339 --> n150
	n339 --> n230
	n339 --> n336
	n339 --> n337
	n339 --> n232
	n230 --> n232
	n336 --> n151
	n336 --> n337
Loading

zcash_encoding is a bit on-the-fence here; the protocols depend on it but only currently inside zcash_primitives where transaction encodings are specified. If we moved the transaction encoding parts into the protocol crates (i.e. a step further than #736), then zcash_encoding would be unambiguously a "component crate" like zcash_note_encryption.

@str4d
Copy link
Contributor Author

str4d commented Jan 30, 2023

Here is how I think I'd like the dependency graph to look after #738 (removing a few of the direct dependencies that can be represented via transitive paths for clarity):

graph TD
	subgraph components
	n232[zcash_note_encryption]
	n150[equihash]
	n151[f4jumble]
	n337[zcash_encoding]
	end

	subgraph protocols
	sapling[sapling-crypto]
	n230[orchard]
	end

	subgraph main
	n336[zcash_address]
	n338[zcash_client_backend]
	n339[zcash_primitives]
	n340[zcash_proofs]
	n341[zcash_client_sqlite]
	end

	n341 --> n338
	n338 --> n339
	n338 --> n340
	n340 --> n339
	n339 --> n150
	n339 --> sapling
	n339 --> n230
	n339 --> n336
	n339 --> n337
	sapling --> n232
	n230 --> n232
	n336 --> n151
	n336 --> n337
Loading

@nathan-at-least
Copy link
Contributor

I came to this issue tracker to ask what defines ./components crates and suggest documenting the distinction in the README. If we're splitting them out into a distinct workspace repo, can I request the README for that new repo clarifies the category.

Is it a correct definition for @str4d's preferred grouping that "components are crates which do not depend on Zcash protocols"?

Another way to think of the definition is what can you build with each set of crates. For the preferred layout is this accurate:

  • main - Zcash MainNet clients/wallets/infra
  • protocols - Zcash protocols in other systems beyond Zcash MainNet
  • components - a broader set of applications / grab bag of utilities useful to the other two but not specific to them

@str4d
Copy link
Contributor Author

str4d commented Sep 27, 2023

Some more thinking about this, and how a single components repo might not be quite the split we want: #995 (comment). This comment lines up somewhat nicely with thje diagram in #768 (comment), if we consider the potential split between components to be "crates that the protocols depend on" (zcash_note_encryption) vs "crates that implement some small self-contained functionality that we don't need to do much maintenance to" (equihash, f4jumble, zcash_encoding). If we go the step further described above, and move some encoding logic into the protocol crates, then maybe we group zcash_encoding with zcash_note_encryption.

@str4d
Copy link
Contributor Author

str4d commented Sep 27, 2023

So maybe this is the split to go for:

graph TD
	subgraph standalone_components
	nEqH[equihash]
	nF4J[f4jumble]
	end

	subgraph protocol_components
	nZEnc[zcash_encoding]
	nZNE[zcash_note_encryption]
	end

	subgraph protocols
	sapling[sapling-crypto]
	orchard[orchard]
	end

	subgraph main
	nZAddr[zcash_address]
	nZPrim[zcash_primitives]
	nZProof[zcash_proofs]
	nZCB[zcash_client_backend]
	nZCS[zcash_client_sqlite]
	end

	nZCS --> nZCB
	nZCB --> nZPrim
	nZCB --> nZProof
	nZProof --> nZPrim
	nZPrim --> nEqH
	nZPrim --> sapling
	nZPrim --> orchard
	nZPrim --> nZAddr
	nZPrim --> nZEnc
	sapling --> nZNE
	orchard --> nZNE
	nZAddr --> nF4J
	nZAddr --> nZEnc
Loading

@str4d
Copy link
Contributor Author

str4d commented Sep 27, 2023

Consider also that zcash_proofs is basically just the Sprout and Sapling circuits, extracted so that people could depend on zcash_primitives without depending on the circuit code. However, the use cases for that split were hardware wallets, none of which have gone in the direction of depending on zcash_primitives. We also now have orchard which requires that we depend on the circuit code.

Given that #738 will end up moving the Sapling circuit logic out of zcash_proofs, that would leave it only containing the Sprout circuit. So we could get rid of zcash_proofs entirely by moving the Sprout circuit into its own protocol crate, and then zcash_proofs is either completely vestigial, or only contains the logic for loading the relevant Zcash parameter files from disk:

graph TD
	subgraph standalone_components
	nEqH[equihash]
	nF4J[f4jumble]
	end

	subgraph protocol_components
	nZEnc[zcash_encoding]
	nZNE[zcash_note_encryption]
	end

	subgraph protocols
	sprout[sprout]
	sapling[sapling-crypto]
	orchard[orchard]
	end

	subgraph main
	nZAddr[zcash_address]
	nZPrim[zcash_primitives]
	nZProof[zcash_proofs]
	nZCB[zcash_client_backend]
	nZCS[zcash_client_sqlite]
	end

	nZCS --> nZCB
	nZCB --> nZPrim
	nZProof --> nZPrim
	nZPrim --> nEqH
	nZPrim --> sprout
	nZPrim --> sapling
	nZPrim --> orchard
	nZPrim --> nZAddr
	nZPrim --> nZEnc
	sapling --> nZNE
	orchard --> nZNE
	nZAddr --> nF4J
	nZAddr --> nZEnc
Loading

Then zcash_primitives itself becomes ripe for refactoring, in particular to take the key tree logic that is currently spread between it and zcash_client_backend and move that into a separate crate that depends on zcash_address (as it would be providing a concrete instantiation of the key tree logic, rather than the "bring your own backend" approach of zcash_address).

@str4d
Copy link
Contributor Author

str4d commented Dec 7, 2023

In #1041 we moved zcash_note_encryption into its own crate, so I think the pattern going forward is likely to be that for protocol components we just create standalone repositories.

@nuttycom
Copy link
Contributor

nuttycom commented Feb 2, 2024

#1140, #1142 and #1143 are intended to produce this result:

%%{init: {"flowchart": {"defaultRenderer": "elk"}} }%%

graph TD
	subgraph standalone_components
	nEqH[equihash]
	nF4J[f4jumble]
	nZEnc[zcash_encoding]
	end

	subgraph protocol_components
	nZNE[zcash_note_encryption]
	nZip32[zip32]
	nZSpec[zcash_spec]
	end

	subgraph protocols
	sprout[sprout]
	sapling[sapling-crypto]
	orchard[orchard]
	end

	subgraph main
	nZAddr[zcash_address]
	nZPrim[zcash_primitives]
	nZProof[zcash_proofs]
	nZCB[zcash_client_backend]
	nZCS[zcash_client_sqlite]
	nZProto[zcash_protocol]
	nZip321[zip321]
	end
	
	subgraph wallets
	nLightFFI[zcash-light-client-ffi]
	nSwiftSDK[zcash-swift-wallet-sdk]
	nAndroidSDK[zcash-android-wallet-sdk]
	nZashiIOS[zashi-ios]
	nZashiAndroid[zashi-android]
	end

	nZCS --> nZCB
	nZCB --> nZPrim
	nZProof --> nZPrim
	nZPrim --> nZProto
	nZCB --> nZip321
	nZip321 --> nZAddr
	nZAddr --> nZProto
	nZip321 --> nZProto
	nZPrim --> nEqH
	nZPrim --> sprout
	nZPrim --> sapling
	nZPrim --> orchard
	nZPrim --> nZEnc
	sapling --> nZNE
	sapling --> nZip32
	sapling --> nZSpec
	orchard --> nZNE
	orchard --> nZip32
	orchard --> nZSpec
	nZAddr --> nF4J
	nZAddr --> nZEnc
	nLightFFI --> nZCS
	nSwiftSDK --> nLightFFI
	nZashiIOS --> nSwiftSDK
	nAndroidSDK --> nZCS
	nZashiAndroid --> nAndroidSDK
Loading

@nuttycom
Copy link
Contributor

This is now in an acceptable state; we no longer have circular dependencies that result in cycles in the repository graph.

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

No branches or pull requests

3 participants