Flexible provenance tracking for state commitments #3527
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This puts management of the compact block where it belongs, in the compact block crate.
This allows the compact block handling to be folded into the top-level application crate. It was only being used for gas scheduling, but that can just as easily be a set of constants instead.
This commit removes the previous `NoteSource`, a stuffed transaction hash, with a new, extensible `CommitmentSource`, and makes managing commitment sources the responsibility of the SCT component that manages state commitments. This allows several important use cases: - We can track provenance of funds as they enter the chain, by recording the sender address, packet number, and channel for `NotePayloads` created by the ICS20 handler. This is particularly important because there is no "transaction" to download to fetch extended information there. - Clients have an easier time deciding whether they need to do an extended fetch of block data, they just need to see if any detected payload was from a `Transaction` source. - We can save space in the compact block by "dehydrating" the transaction sources and removing the transaction hashes. Clients _already_ have to fetch a full block's worth of transactions, because nullifiers don't have source info attached, so we gain nothing by including detailed source info in the `CompactBlock`. - Clients have an easier time working with Penumbra data, since there's no more ad-hoc stuffing.
Since any kind of state fragment can be nullified, this should be the responsibility of the SCT component, not the shielded pool.
Replicating all the type parameters everywhere is quite dangerous, since if any of them are mistyped, the query will silently fail.
hdevalence
force-pushed
the
provenance
branch
from
50d2eff to
5ce08e9
Compare
hdevalence
had a problem deploying
to
smoke-test
GitHub Actions
Failure
hdevalence
had a problem deploying
to
smoke-test
GitHub Actions
Failure
hdevalence
had a problem deploying
to
smoke-test
GitHub Actions
Failure
hdevalence
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
hdevalence
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
hdevalence
commented
Dec 19, 2023
Comment on lines
-16
to
-25
| /// Queries the state commitment tree's block anchor for a given height. | ||
| BlockAnchor { | ||
| /// The height to query. | ||
| height: u64, | ||
| }, | ||
| /// Queries the state commitment tree's epoch anchor for a given epoch index. | ||
| EpochAnchor { | ||
| /// The epoch to query. | ||
| epoch: u64, | ||
| }, |
There was a problem hiding this comment.
This is functionality that should really be part of an indexer / explorer, rather than asking every full node to keep the data around in case someone wants to run this command against a random fullnode.
hdevalence
commented
Dec 19, 2023
Comment on lines
+149
to
+153
|
|
||
| state_tx | ||
| .finish_block(state_tx.app_params_updated()) | ||
| .await | ||
| .expect("must be able to finish compact block"); |
There was a problem hiding this comment.
Finishing the compact block is now the responsibility of the compact block component.
hdevalence
commented
Dec 19, 2023
| @@ -60,6 +62,7 @@ async fn spend_happy_path() -> anyhow::Result<()> { | |||
| spend.check_stateless(transaction_context).await?; | |||
| spend.check_stateful(state.clone()).await?; | |||
| let mut state_tx = state.try_begin_transaction().unwrap(); | |||
| state_tx.put_mock_source(1u8); | |||
There was a problem hiding this comment.
This has to be inserted now that the SCT logic tracks provenance, otherwise execution will fail, because in the mock setting, we're only running a single action in isolation, which wouldn't pick up the transaction-level source setting.
hdevalence
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
|
Demo of this support in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3349
This PR removes the previous
NoteSource, a stuffed transaction hash, witha new, extensible
CommitmentSource, and makes managing commitment sources theresponsibility of the SCT component that manages state commitments. This
allows several important use cases:
We can track provenance of funds as they enter the chain, by recording the
sender address, packet number, and channel for
NotePayloadscreated by theICS20 handler. This is particularly important because there is no "transaction"
to download to fetch extended information there.
Clients have an easier time deciding whether they need to do an extended
fetch of block data, they just need to see if any detected payload was from a
Transactionsource.We can save space in the compact block by "dehydrating" the transaction
sources and removing the transaction hashes. Clients already have to fetch
a full block's worth of transactions, because nullifiers don't have source info
attached, so we gain nothing by including detailed source info in the
CompactBlock.Clients have an easier time working with Penumbra data, since there's no more
ad-hoc stuffing.
In addition, it does various other cleanup along the way.