Skip to content

feat(crypto): optimize zksnark config #6371

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

Open
wants to merge 9 commits into
base: release_v4.8.1
Choose a base branch
from

Conversation

Federico2014
Copy link
Contributor

What does this PR do?
Close #6334

if (isShieldedTransaction(trx.getInstance()) && !Args.getInstance()
.isFullNodeAllowShieldedTransactionArgs()) {
return true;
if (isShieldedTransaction(trx.getInstance()) && !chainBaseManager.getDynamicPropertiesStore()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how can we support shielded transaction through config item committee.xxx ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support shielded transactions, we need to turn on ALLOW_SHIELDED_TRANSACTION proposal. However, the corresponding code is commented out.

@@ -24,11 +24,6 @@ private void init() {
public static void librustzcashInitZksnarkParams() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be optimized so that it is only initialized once in the JVM? Currently, a large number of unit tests repeatedly generate related files, causing insufficient space in the tmp folder.

Image Image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will fix it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private static final AtomicBoolean initialized = new AtomicBoolean(false);
If (initialized.compareAndSet(false, true)){
  do InitZksnark
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After optimization, the initZksnarkParams method is invoked only 6 times during tests, significantly fewer than before.

@@ -1030,10 +1030,6 @@ public static void setParam(final Config config) {
config.hasPath(Constant.NODE_SHIELDED_TRANS_IN_PENDING_MAX_COUNTS) ? config
.getInt(Constant.NODE_SHIELDED_TRANS_IN_PENDING_MAX_COUNTS) : 10;

if (PARAMETER.isWitness()) {
PARAMETER.fullNodeAllowShieldedTransactionArgs = true;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic should not be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be removed. The witness can allow the shieldedTransactionApi freely.

@Federico2014 Federico2014 force-pushed the fix/optimize_zksnark_config branch from 3a031e3 to 7676699 Compare July 8, 2025 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

Optimize the configuration switch used for zkSNARK and shielded transaction
7 participants