Network reset for circuit changes #3775
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
* adds transactionHashToBlockHash chain index adds an index mapping transaction hash to block hash for all transactions on the main chain. the index will support looking up transactions on the chain using only the transaction hash. this lookup currently requires a block hash because the transactions store is keyed by block hash. the index is a datastore with a TransactionHash (i.e., Buffer) key and a BlockHash (i.e., Buffer) value. entries are inserted into the index for each transaction when a block is connected to the main chain (in saveConnect). entries are deleted from the index for each transaction when a block is disconnected from the main chain (in saveDisconnect). * modify tests --------- Co-authored-by: danield9tqh <[email protected]>
* makes blockHash optional in chain/getTransaction requests after adding an index from transaction hash to block hash in #3761 we no longer need to include a block hash in requests to chain/getTransaction. instead we can use the index to look up the hash of the block on the main chain that includes that transaction, if any. - defines 'getHashByTransactionHash' to look up a block hash in 'transactionHashToBlockHash' - changes 'blockHash' to an optional request parameter - adds route tests for chain/getTransaction * fixes lint * changes method name to include 'Block' for clarity
* Add nonce to Asset, use blake2s to get asset identifier This moves away from using a pedersen hash for asset info -> asset id, instead using a blake2s hash. The nonce is added because not every blake2s hash maps to a valid point, so this allows the code to silently retry until it finds a valid point for the given owner, name, metadata. * Type refactoring - Introduce AssetIdentifier as a distinct type rather than using it as an alias of [u8; 32]. This allows us to utilize the type system so that we know if we have an instance of an AssetIdentifier, it is valid as an id and that it derives to a valid generator. Requires a little more effort on instantiation, but makes the usage of it much cleaner. - Change Asset Generator to ExtendedPoint. When we instantiate an Asset, we hash the asset info into the AssetIdentifier and check that this AssetIdentifier hashes to a valid point in the prime group. However, we continue utilizing it as an ExtendedPoint type to make validation of asset id to asset generator in the circuit easier. - Introduce Value Commitment Generator. This is the SubgroupPoint version of the Asset Generator. This isn't used yet, but will be used for the value commitments and value balancing. - Change Note to take AssetIdentifier instead of Asset Generator. This is because the encrypted note contents will contain the AssetIdentifier now, not the generator. This commit ended up being a lot of things together that I would have preferred to keep separate, but I started in the wrong place and untangling it to be a useful commit while also keeping the compiler happy ended up being a lot of work. * Tweak length consts * Move AssetIdentifier into its own file * Start using Value Commitment Generators We should be using the SubgroupPoint versions of the generators for value balancing. This is incompatible with the circuits until those are updated. * add a quick unit test for Asset::new_with_nonce * Update mint asset circuit We use the blake2s hash instead of the pedersen hash, as outlined in the Asset changes. We also added the nonce to the pre-image. We are not explicitly checking that the asset id hashes to a valid generator point in the circuit. This is unnecessary, because the mint information is public. Since we introduced the AssetIdentifier struct, which implicitly checks that any asset id instantiated is a valid generator point, we get this for free when the transaction is serialized into the Rust as part of calling `batch_verify_transactions`. This also happens for the burn. * Update spend circuit We clear the cofactor and assert that the value commitment generator is not small order. We're also using the ValueCommitment's Asset Generator instead of passing it in seperately, which is unnecessary. * Update output circuit In addition to the checks added in the previous commit for spend, which apply here since both circuits use the expose_value_commitment function, we are also witnessing the asset id deriving the asset generator, and proving that it is the same one that is being used in the value commitment * update temporary params * Add asset.nonce for Typescript, regen NAPI bindings * typescript asset store and test updates to handle nonce * change temporary / dev genesis blocks * update typescript test fixtures * feat(rust,zkp): Update personalizations for PRF and value commitment (#3668) * feat(rust,zkp): Update personalizations for PRF and value commitment * fix(ironfish): Update fixtures * feat(rust,zkp): Move asset id check from circuit to consensus (#3686) * feat(rust,zkp): Move asset id check from circuit to consensus * feat(rust,zkp): Add `GH_FIRST_BLOCK` * test(ironfish): Update fixtures * test(ironfish): Update createTransaction fixture * test(ironfish): Fix fixtures after a rebuild * feat(rust): Add `partial_verify` when building spends and outputs (#3687) * feat(ironfish,rust): Update asset metadata length from 77 to 96 (#3703) * fix(rust): Instantiating an asset only calls the hash to point function once (#3705) When instantiating an asset, it was calling `asset_hash_to_point`, and then converting the hash array into an `AssetIdentifier`, which also calls `asset_hash_to_point`. Because of this, this function does not need to call it anymore. * chore(zkp): Remove unused pedersen personalization (#3706) * update circuits for trusted setup code * chore(ironfish): Revert fixtures --------- Co-authored-by: Mat <[email protected]> Co-authored-by: mat-if <[email protected]> Co-authored-by: danield9tqh <[email protected]>
rohanjadvani
approved these changes
Apr 12, 2023
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.
Summary
Network reset including circuit changes from #3744
Testing Plan
Unit tests, local testing + simulation testing
Documentation
Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.
Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and
what additional work is required, if any.