input-output-hk / mithril

Stake-based threshold multi-signatures protocol
https://mithril.network
Apache License 2.0
115 stars 36 forks source link

Fix: make prover not certify transactions stored but not certified yet #1721

Closed jpraynaud closed 1 month ago

jpraynaud commented 1 month ago

Content

This PR includes a fix to the Cardano transactions prover to avoid certifying transactions stored in the database, but not already certified. This was causing the error: Proof and certificate don't match when trying to verify this type of transactions instead of marking them as not certified.

Benchmarks

Screenshot from 2024-06-06 15-23-44

Total Requests Concurrency Transactions/Request Pool (x50) : Request/s Pool (x50) + Fix not yet certified transactions: Request/s
1000 100 1 273.78 275.95
1000 100 2 201.6 200.65
1000 100 3 143.14 150.64
1000 100 4 115.41 124.02
1000 100 5 91.15 103.31
1000 100 6 80.58 87.72
1000 100 7 74.23 77.33
1000 100 8 65.05 68.08
1000 100 9 58.78 61.77
1000 100 10 52.51 58.13

Running on Linux / 8 cores / 64 GB RAM / 2 TB SSD

Pre-submit checklist

Issue(s)

Relates to #1719

github-actions[bot] commented 1 month ago

Test Results

    3 files  ±0     43 suites  ±0   8m 38s :stopwatch: -1s 1 013 tests +1  1 013 :white_check_mark: +1  0 :zzz: ±0  0 :x: ±0  1 111 runs  +1  1 111 :white_check_mark: +1  0 :zzz: ±0  0 :x: ±0 

Results for commit 986a3cc2. ± Comparison against base commit ac3eb960.

This pull request removes 2 and adds 3 tests. Note that renamed tests count towards both. ``` mithril-aggregator ‑ services::prover::tests::compute_proof_for_one_set_of_three_known_transactions mithril-aggregator ‑ services::prover::tests::compute_proof_for_one_set_of_three_known_transactions_and_two_unknowns ``` ``` mithril-aggregator ‑ services::prover::tests::cant_compute_proof_for_not_yet_certified_transaction mithril-aggregator ‑ services::prover::tests::compute_proof_for_one_set_of_three_certified_transactions mithril-aggregator ‑ services::prover::tests::compute_proof_for_one_set_of_three_certified_transactions_and_two_unknowns ```

:recycle: This comment has been updated with latest results.