Skip to content

Private state update metadata and migration#354

Closed
lucassaldanha wants to merge 24 commits into
besu-eth:masterfrom
lucassaldanha:updated-migration
Closed

Private state update metadata and migration#354
lucassaldanha wants to merge 24 commits into
besu-eth:masterfrom
lucassaldanha:updated-migration

Conversation

@lucassaldanha
Copy link
Copy Markdown
Contributor

@lucassaldanha lucassaldanha commented Feb 2, 2020

PR description

• Updated the private state storage with new data structures that aren't backwards compatible.
• Added PrivateStateRootResolver that handles public chain forks and their impact on the private state
• Created migration logic that will process previous private txs and re-create the data in the update 1.4.x format.
• Migration is based on the schema version, defined in the PrivateStateStorage
• Added flag that user needs to "opt-in" to migrate the data
• Remove restriction for "latest" block from priv_call

@lucassaldanha lucassaldanha self-assigned this Feb 2, 2020
@lucassaldanha lucassaldanha added the privacy private transactions label Feb 2, 2020
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
final PrivacyParameters privacyParameters = privacyParametersBuilder.build();

if (isPrivacyEnabled) {
preSynchronizationTaskRunner.addTask(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is that how it is done in besu? Why not check whether migratePrivateDatabase is true and then add the task?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because part of the task is to evaluate the different triggering conditions. And one of the conditions is: you have a private db that needs migration but you don't have the migratePrivateDatabase flag enabled.

import org.hyperledger.besu.controller.BesuController;

/**
* All PreSynchronizationTask insatnces execute after the {@link BesuController} instance in {@link
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/insatnces/instances/

this.privacyParameters = privacyParameters;

final PrivateStateRootResolver privateStateRootResolver =
new PrivateStateRootResolver(privacyParameters.getPrivateStateStorage());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

inline?

return privacyParameters;
}

public PrivateNonceProvider getPrivateNonceProvider() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IntelliJ says this method is never called. Do we need it? And the privateNonceProvider?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe we don't need getPrivateNonceProvider() anymore. We definitely need PrivateNonceProvider though. :)
I have removed the method.

final BlockHeader blockHeader,
final List<Transaction> transactions,
final List<BlockHeader> ommers) {
final PrivacyGroupHeadBlockMap privacyGroupHeadBlockHash =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/privacyGroupHeadBlockHash/privacyGroupHeadBlockMap/

privateStateRootResolver, blockchain, STATE_ROOT_AFTER_TRANSACTION_APPENDED_TO_EMTPY_STATE);

final String secondForkBlockStateRoot =
"0xd35eea814b8b5a0b12e690ab320785f3a33d9685bbf6875637c40a64203915da";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

constant


// Add another private transaction
final String thirdForkBlockStateRoot =
"0xe22344ade05260177b79dcc6c4fed8f87ab95a506c2a6147631ac6547cf44846";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

constant

parentBlock);
}

private BlockDataGenerator.BlockOptions getBlockOptionsNoTransaction(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this block will have a transaction!!! Fix the name!

.setEnclaveUrl(enclave.clientUrl())
.setEnclaveFactory(new EnclaveFactory(Vertx.vertx()))
.build();
privacyParameters.setEnclavePublicKey("A1aVtMxLCUHmBVHXoZzzBgPbW/wj5axDpW9X8l91SGo=");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use the constant ENCLAVE_PUBLIC_KEY

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In lines 154 and 410 we are using the String for that key as a parameter. Can we have a constant for the String as well (ENCLAVE_PUBLIC_KEY_STRING) to make clear what we are doing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can use ENCLAVE_PUBLIC_KEY.toBase64String()

@@ -0,0 +1,540 @@
/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would be nice if we could create constants for the "magic" numbers, but if we cannot do that we should provide some kind of clue where these numbers come from ...

pinges and others added 2 commits February 7, 2020 08:18
return Optional.empty();
}

final MutableWorldState publicWorldState =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we are simulating this private transaction we need to use the public state at the end of the public Tx just before the PMT.

Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
This reverts commit 118c7e5

Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>
@lucassaldanha
Copy link
Copy Markdown
Contributor Author

Superseeded by #404

@lucassaldanha lucassaldanha deleted the updated-migration branch February 17, 2020 02:18
siladu pushed a commit to siladu/besu that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

privacy private transactions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants