zkopru-network / zkopru Goto Github PK

Is your feature request related to a problem? Please describe.

By the issue #34, a Zk account will have 64 bytes public address data which is a set of
(P, N) where P is poseidon(p*G, n) and N is n*G. (p is spending key and n is nullifier seed)

In this context, Base58Check is a good option to shorten the address for the readability.

P: 32bytes = poseidon(p*G, n)
N: 32bytes = encode(n*G)
checksum: 4bytes = keccak256(P, N)
totalBytes: 68 bytes = P + N + checksum
address = base58(totalBytes)

Describe the solution you'd like
N/A

Describe alternatives you've considered
Base58 without checksum

Additional context
#34

1. compile circuits and generate snark keys.
2. prepare 4 zk accounts(Alice, Bob, Carl, and Coordinator)
3. deploy ERC20 and ERC721 contracts, then airdrop assets to given accounts.
4. deploy zkopru contracts and register verification keys (normal case integration test will have 30 seconds of challenge period).
5. launch coordinator.
6. prepare three CLI wallets for Alice, Bob, and Carl.
7. Alice deposits Ethers to the layer 2 repetitively.
8. Bob deposits ERC20s and Ethers to the layer 2 repetitively.
9. Carl deposits ERC721s and Ethers to the layer 2 repetitively.
10. Alice sends Ethers to Bob and Carl repetitively.
11. Bob trades ERC20/ERC721 with Carl repetitively.
12. Bob trades ERC721/Ethers with Alice repetitively.
13. Carl trades ERC20/Ethers with Alice repetitively.
14. Alice withdraws ERC20 and ERC721 repetitively using her second account.
15. The coordinator creates, proposes, and finalizes blocks.
16. The coordinator withdraws fees.

Is your feature request related to a problem? Please describe.
Zkopru currently does not handle the re-org situation. The reorg should affect its database and its sync status.

Describe the solution you'd like
Mark followings as canceled

• NewProposal
• NewDeposit
• NewMassDeposit
• Finalization

Describe alternatives you've considered
N/A

Additional context
N/A

Is your feature request related to a problem? Please describe.
Migration feature is implemented in the contract but there's no impl in client level.

Describe the solution you'd like
Add 'migration' menu to the CLI application

Describe alternatives you've considered
N/A

Additional context
N/A

uint is an alias of uint256. Both uint and uint256 appear in the codebase. For consistency, I suggest sticking to uint256 everywhere.

- written by @weijiekoh

Migrants are "addresses allowed to migrate from".

The only time that the coordinator can modify the Layer2.allowedMigrants storage mapping is during the setup phase via the setup wizard. The 15_setup.js script, however, does not invoke wizard.allowMigrants (as it is commented out).

-written by @weijiekoh

It should be able to send 5 types of zk transactions

• Alice sends Ethers to Bob
• Alice sends ERC20s to Bob
• Alice sends NFTs to Bob
• Alice sends ERC20 to swap with Bob's NFT
• Alice merges her UTXOs into one UTXO

The setup wizard in packages/contracts/migrations/15_setup.js executes a series of functions to store key pieces of data including contract addresses and verifying keys in the proxy contract. The wizard runs completeSetup() at the end of the process, which prevents the wizard from running these functions again. An intention behind this process is to run each function only once.

registerVk(), however, could be run multiple times before completeSetup(). The wizard could technically register a VK of n, m inputs and outputs multiple times. This may not be what the coordinator intends to do, and it is not what the setup script does, but this edge case nevertheless exists.

Moreover, registerVk() appends values to vk.ic, so if it is called more than once, the vk.ic array will grow and this would waste gas. As such, if the wizard wants to use this function to replace a previous VK before the setup is complete, they will not be able to do so anyway.

To prevent this edge case, add a check in registerVk() to prevent the wizard from calling it twice for the same n, m. Alternatively, the setup script should check the contract storage after completeSetup() to ensure that all data, and only correct data, have been stored.

-written by @weijiekoh

Users will lose the detail information about own notes if they delete the local database. So it would be great if we can offer some cloud based back up solutions to keep the notes with some encryption.

The file packages/contracts/contracts/libraries/SNARKs.sol defines a VerifyingKey struct, and zkSNARKs() uses it as the type of one of its parameters. The registerVk() function in packages/contracts/contracts/ZkOptimisticRollUp.sol, however, does not use VerifyingKey. For the sake of consistency and simplicity, it should do so.

- written by @weijiekoh

UTXO tree root & every each zk transaction should use the snark field instead of uint256. The challenge system will have some conditions to check this issue.

Is your feature request related to a problem? Please describe.
If we use the spec #34, there exists a possibility to generate duplicated nullifiers with multiple UTXO trees system.
Therefore, it is better to manage a single UTXO tree to get rid of the unfortunate possibility and also to increase the anonymity set.

Describe the solution you'd like
increase tree depth 32 to 64

Describe alternatives you've considered
Make nullifier includes tree's index. But it hurts the privacy.

Additional context
N/A

Is your feature request related to a problem? Please describe.
Dryrun of challenge transactions using call() functions can convince the block's validity

Describe the solution you'd like
Run a standalone full node that only the coordinator can access (in same private network), and use call functions to test challenges.

Describe alternatives you've considered
Using trusted 3rd parties' full node. But it'll be not stable.

Additional context
https://github.com/thehubbleproject

Describe the bug
Testing on a new machine doesn't work well.

To Reproduce

git clone https://github.com/zkopru-network/zkopru && cd zkopru
yarn build
yarn test

Expected behavior
exit code 0

Screenshots

Desktop (please complete the following information):

• OS: Linux, Ubuntu focal
• Version 1.0.0-beta.2

Smartphone (please complete the following information):
N/A

Additional context
N/A

The solidity library github.com/wanseob/smt-rollup is fixed to use 256 depth SMT.
But because nullifier has 254 bit for its size, it is better to use 254 depth SMT.

coordinator application should have those following features

1. manage full node
2. manage tx pool
3. build a new block
4. propose a new block
5. finalize blocks
6. run express based HTTP server to receive transactions (in the future, it can be replaced by libp2p or something others)
7. run a bootstrap node to provide bootstrap data to wallet clients

Message not deliveredThere was a problem delivering your message to [email protected]. See the technical details below.LEARN MORE | Message not deliveredThere was a problem delivering your message to [email protected]. See the technical details below.LEARN MORE | Message not delivered | There was a problem delivering your message to [email protected]. See the technical details below. | LEARN MORE
-- | -- | -- | -- | --
Message not deliveredThere was a problem delivering your message to [email protected]. See the technical details below.LEARN MORE | Message not delivered | There was a problem delivering your message to [email protected]. See the technical details below. | LEARN MORE
Message not delivered
There was a problem delivering your message to [email protected]. See the technical details below.
LEARN MORE
The response was:The recipient server did not accept our requests to connect. Learn more at https://support.google.com/mail/answer/7720 [zkopru.network 104.198.14.52: timed out]

Tried from two different emails

Is your feature request related to a problem? Please describe.
The sender creates a UTXO for the recipient, and therefore the sender can track the 1st transaction after the recipient receives the UTXO.

In version(0.7.0), Zkopru computes the hash of the UTXO with

var intermediate_hash = poseidon(ether, pub_key.x, pub_key.y, salt)
var result_hash = poseidon(intemediate_hash, token_address, erc20, nft)

Describe the solution you'd like

The recipient create a zk-tx public key with the following logic and share to others.

var zk_public_key = poseidon(pub_key.x, pub_key.y, nullifier_seed)

We can generate the UTXO with following steps:

var asset_hash = poseidon(ether, token_address, erc20, nft)
var utxo_hash = poseidon(zk_public_key, asset_hash, salt)

Then finally the nullifier becomes

var nullifier = poseidon(nullifier_seed, leaf_index)

Describe alternatives you've considered
MPC protocol can help this, but non-interactive way preferred.

Additional context
N/A

Is your feature request related to a problem? Please describe.
Currently, for the instant withdrawal, the original withdrawal note owner should give a signature for the hash of the prepayer's address and the withdrawal note's hash. But just providing a signature independent to the prepayer can help us run an open prepay market for everyone.

Describe the solution you'd like
Modify the hash message for the signature here,
https://github.com/wanseob/zkopru/blob/034ad7b41eca2a9fc0d344a5b5a8a4525e904c96/packages/contracts/contracts/controllers/UserInteractable.sol#L79-L84

Describe alternatives you've considered
N/A

Additional context
N/A

Nanosql seems not maintained for a while

The current system is using Prisma for its database layer.

But the current version of Prisma has a lack of transaction support and it causes the bottle-neck in the merkle tree update process. The following two cases especially need to be updated with that feature.

1. SMT update
   https://github.com/wanseob/zkopru/blob/e073d6fe03ce0449d10ce9a2537e514c641fe3f3/packages/tree/src/nullifier-tree.ts#L174-L187

2. Cached nodes for merkle proof
   https://github.com/wanseob/zkopru/blob/e073d6fe03ce0449d10ce9a2537e514c641fe3f3/packages/tree/src/light-rollup-tree.ts#L474-L492

related Prisma issues:

https://github.com/prisma/prisma-client-js/issues/349
prisma/prisma-client-js#667
prisma/specs#356

Is your feature request related to a problem? Please describe.
N/A

Describe the solution you'd like
N/A

Describe alternatives you've considered
N/A

Additional context
iden3/circomlib@86c6a2a

Is your feature request related to a problem? Please describe.
snarkjs 0.1.x version calculates witness with javascript, but snarkjs 0.3.x calculates them with wasm code. Migrating the library will definitely enhance the tx proof generation speed.

Describe the solution you'd like
Use the latest 0.3.x version of snarkjs.

Describe alternatives you've considered
N/A

Additional context
https://github.com/iden3/snarkjs

Version 0.7 of the Solidity compiler may introduce breaking changes.

The contracts currently use this version pragma: pragma solidity >= 0.6.0;

As such, I suggest that we change the version pragma to pragma solidity ^0.6.0;.

- written by @weijiekoh

Deserializer.sol makes heavy use of assembly code and hardcoded values to indicate specific byte-positions of data. It would be helpful to add detailed comments to explain what the assembly code does and what each hardcoded value refers to.

-written by @weijiekoh

Is your feature request related to a problem? Please describe.
Logging is too unorganized currently, so we need to clean up that.

Describe the solution you'd like

• new log format using pino-pretty
• clean up logs
• add log trace option to the cli config

Describe alternatives you've considered
N/A

Additional context
N/A

Is your feature request related to a problem? Please describe.
N/A

Describe the solution you'd like
Add "Dev Sequence" part to the CONTRIBUTING.md file

Describe alternatives you've considered
N/A

Additional context
Refer this file.
packages/integration-test/utils/playground-setup.ts

The cli wallet application based on yargs should support those features

1. HD wallet
2. show spendable assets
3. deposit l1 assets to l2
4. withdraw l2 assets to l1
5. send l2 assets to l2

• Show spendable utxos & sum of balances
• Build zk tx
• Gen block including zk txs
• Receive zk tx

• verifyTx functions seems not working properly
  https://github.com/wanseob/zkopru/blob/ab68ef8f96f196e1759903d47ee7d4d5eefe2a7d/packages/core/src/snark-verifier.ts#L15
• Releated source codes:
  • https://github.com/wanseob/zkopru/blob/ab68ef8f96f196e1759903d47ee7d4d5eefe2a7d/packages/contracts/contracts/libraries/SNARKs.sol#L16
  • https://github.com/wanseob/zkopru/blob/ab68ef8f96f196e1759903d47ee7d4d5eefe2a7d/packages/contracts/contracts/storage/Layer2.sol#L105

It will include

For developers

• how to setup dev env

• how to run integration tests & unit test

• how to run the manual operation sequence test => should be automated in the integration-test

  • Build circuits and generate verifying keys and proving keys.
  • Copy the generated keys to the migration files.
  • Run ethereum node (example: ganache)
  • Deploy contracts using truffle migrate => it will deploy contracts and register verifying keys
  • Run cli:coordinator with configuration (configure db & layer 1 node, etc)
  • Check registered verification keys are same with the coordinator client's keys
  • Complete setup => now no one can register new verifying keys
  • Check that new vk registrations fail.
  • Run cli:wallet with configuration (configure db & coordinator & layer 1 node)
  • deposit ETH / ERC20 / ERC721
  • transfer ETH / ERC20 / ERC721
  • withdrawl ETH / ERC20 / ERC721
  • call finalize (testing env should have short finalization period)
  • withdraw notes to layer 1

• how to browse the database of cli:coordinator or cli:wallet

• where the logs are stored

• how to run development environment for cli:coordinator (docker-compose)

• how to run development environment for cli:wallet (docker-compose)

• zkopru.network => coordinator url & snark keys CDN

• coordinator api document

• contract api document

• how to run coordinator dashboard

Wiki

• Data types
• Serialization
• Sub-tree based Merkle tree roll-up algorithm

packages/integration-test/tests/index.test.ts has a bunch of integration test cases of the system.

You can append more to do items to the file.

ex)

describe('10: Challenge', () => {
  describe('fraud', () => {
    it.todo('coordinator creates an invalind utxo roll up')
  })
  describe('watchdog', () => {
    it.todo('alice catches the fraud and submit a challenge')
  })
  describe('slash', () => {
    it.todo('coordinator gets slashed and the block gets invalidated')
    it.todo('alice gets the challenge reward')
  })
  describe('revert', () => {
    it.todo('every clients should update the revert')
  })
})