Skip to content
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.

Sign up for GitHub

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

Jump to bottom

soft fork: implement a way to change the circuit of a chain #1205

Merged
merged 5 commits into from
Dec 5, 2023
Merged

soft fork: implement a way to change the circuit of a chain #1205

merged 5 commits into from
Dec 5, 2023

Conversation

altergui
Copy link
Contributor

@altergui altergui commented Nov 27, 2023 •
edited
Loading

(see commit messages, please don't squash)

p4u
p4u reviewed Nov 28, 2023
View reviewed changes
vochain/cometbft.go Outdated Show resolved Hide resolved
p4u
p4u reviewed Nov 28, 2023
View reviewed changes
vochain/genesis/genesis.go Outdated Show resolved Hide resolved
@altergui altergui force-pushed the fork/circuit-tags branch 3 times, most recently from 33926fb to 3240d24 Compare November 28, 2023 16:41
altergui
altergui commented Nov 28, 2023
View reviewed changes
crypto/zk/circuit/config.go Outdated Show resolved Hide resolved
@altergui altergui force-pushed the fork/circuit-tags branch 3 times, most recently from 4d9eb9c to f7dd973 Compare November 28, 2023 19:32
@altergui
Copy link
Contributor Author

TestVoteCheckZkSNARK is failing, any ideas @lucasmenendez ?

2023-11-28T20:32:46.45+01:00 ERR vochain/cometbft.go:210 > checkTx error="voteTx: proof not valid: CSP bundle signature does not match"
2023-11-28T20:32:46.451+01:00 ERR vochain/app.go:234 > rejected tx error="voteTx: proof not valid: CSP bundle signature does not match"
--- FAIL: TestVoteCheckZkSNARK (5.32s)
    transaction_zk_test.go:115: 
        error:
          got non-nil error
        got:
          e"expired sik root provided, generate the proof again"
        stack:
          /home/user/src/vocdoni/vocdoni-node/vochain/transaction_zk_test.go:115
            c.Assert(err, qt.IsNil)

@altergui altergui force-pushed the fork/circuit-tags branch 6 times, most recently from c8d8f8d to 451e07a Compare November 28, 2023 23:39
@altergui altergui linked an issue Dec 4, 2023 that may be closed by this pull request
soft-fork to incorporate the result from the zk-voceremony #1196
Closed
@altergui altergui force-pushed the fork/circuit-tags branch 3 times, most recently from 90176ce to 7dc0342 Compare December 4, 2023 18:37
@coveralls
Copy link

coveralls commented Dec 4, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 7099611641

  • 149 of 187 (79.68%) changed or added relevant lines in 21 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.1%) to 61.958%
Changes Missing Coverage Covered Lines Changed/Added Lines %
crypto/ethereum/vocdoni_sik.go 4 5 80.0%
crypto/zk/circuit/inputs.go 3 4 75.0%
util/zk.go 18 19 94.74%
vochain/transaction/election_tx.go 1 2 50.0%
api/chain.go 14 16 87.5%
apiclient/client.go 3 5 60.0%
config/forks.go 3 5 60.0%
crypto/zk/circuit/config.go 4 6 66.67%
crypto/zk/prover/prover.go 10 13 76.92%
vochain/start.go 7 10 70.0%
Files with Coverage Reduction New Missed Lines %
crypto/zk/prover/prover.go 2 80.85%
Totals Coverage Status
Change from base Build 7046334717: 0.1%
Covered Lines: 14839
Relevant Lines: 23950
💛 - Coveralls

@altergui altergui marked this pull request as ready for review December 4, 2023 18:48
@altergui altergui self-assigned this Dec 4, 2023
@altergui
Copy link
Contributor Author

altergui commented Dec 4, 2023 •
edited
Loading

last steps:

@altergui altergui marked this pull request as draft December 4, 2023 18:58
@altergui altergui force-pushed the fork/circuit-tags branch 2 times, most recently from 45fe77f to 7e686dd Compare December 5, 2023 09:01
@altergui altergui marked this pull request as ready for review December 5, 2023 09:01
@altergui altergui requested a review from p4u December 5, 2023 09:11
@altergui
vochain fork: refactor ZkCircuits handling
27f8863 
* circuit package now has a Global() circuit, and all packages (including TransactionHandler) use that
* circuit/config.go: rename `dev` -> `v0.0.1` and add voceremony as `v1.0.0`
* circuit/config.go: rename `tag` concept into `version`
* circuit/config.go: now ZkCircuitConfig has Version field, drop app.circuitConfigTag
* circuit/config.go: now PublicSignals is a property of each circuit (previously hardcoded in prover)
* prover: use PubSignals from circuit.Global() instead of hardcoded indexes
* api: /chain/info now returns circuitVersion (instead of misspelt cicuitConfigurationTag)
* apiclient: small fix, LoadZkCircuit once on NewHTTPclient instead of every Vote
* testsuite: mount zkCircuits cache dir in test container as well
* vochain/app.go: SetZkCircuit during beginBlock
* add config/forks.go
* circuit: add DownloadArtifacts funcs
  * DownloadArtifactsForChainID
  * DownloadDefaultArtifacts
* NewBaseApplication now calls circuit.DownloadDefaultArtifacts instead of transactionHandler.LoadZkCircuit
* newTendermint now calls circuit.DownloadArtifactsForChainID
@altergui altergui merged commit 1df4860 into main Dec 5, 2023
19 checks passed
@altergui altergui deleted the fork/circuit-tags branch December 5, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p4u p4u p4u approved these changes

@lucasmenendez lucasmenendez Awaiting requested review from lucasmenendez

Assignees

@altergui altergui

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

soft-fork to incorporate the result from the zk-voceremony
3 participants
@altergui @coveralls @p4u