feat: Adding fr compatibility to smt variables api by Sarkoxed · Pull Request #4884 · AztecProtocol/aztec-packages

Sarkoxed · 2024-03-01T12:52:47Z

This pr updates FFTerm and FFITerm classes. Also CMakeLists are changed a little.

FFTerm

Added Fr compatibility. Now you can perform arithmetic operations with symbolic variables using bb::fr rather than strings or FFTerm::Const

FFITerm

Added .mod() method, that performs the operation of taking the remainder on terms. It leads to less constrained system of equations and helps the solver to work faster. However, now you have to do it manually.
Added Fr compatiblitity(same here)
Added xor, $\ge, \gt, \le, \lt$ operations
Changed Constant variable initialization a little

Since you need these classes to behave similarly, they now both have identical methods. However, most of them are just placeholders in FFTerm class.

Tests

Added simple unit tests for both FFTerm and FFIterm to test that all the arithmetic operations are implemented properly.

updt

Terms

changed division method to be more readable
changed the hash value that is used as a dvision symbolic variable
name to md5(Aztec)
Removed the mod operations from inequalities methods in FFITerm
Modified == and != methods in FFITerm so now there won't be any
redundant mod operations

- Now they are compatible with Fr variables - Changed FFIterm constant initialization

Rumata888 · 2024-03-01T13:27:12Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffiterm.cpp

-            this->term = slv->s.mkInteger(tmp);
-        }
-        // this->term = slv->s.mkInteger(tmp); won't work for now since the assertion will definitely fail
+        std::string strvalue = slv->s.mkFiniteFieldElem(t, slv->fp, base).getFiniteFieldValue(); // TODO(alex): works for now


Could you add a more descriptive TODO? What do you want to do here in the future?
Also, we are switching to the TODO format, where we add an issue here (https://github.com/AztecProtocol/barretenberg/issues) with the description and then write TODO(#issue_number)

Rumata888 · 2024-03-01T13:29:29Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffiterm.cpp

-    cvc5::Term res = this->solver->s.mkConst(this->solver->s.getIntegerSort(),
-                                             "fe0f65a52067384116dc1137d798e0ca00a7ed46950e4eab7db51e08481535f2_div_" +
-                                                 std::string(*this) + "_" + std::string(other));
+    cvc5::Term res = Var("fe0f65a52067384116dc1137d798e0ca00a7ed46950e4eab7db51e08481535f2_div_" + std::string(*this) + "_" + std::string(other), this->solver).term;


What is this hash?

Rumata888 · 2024-03-01T13:30:24Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffiterm.cpp

+}
+
+void FFITerm::operator<(const bb::fr& other) const{
+    cvc5::Term lt = this->solver->s.mkTerm(cvc5::Kind::INTS_MODULUS, {this->term, this->modulus});


Why are you doing modular reduction in these?

Rumata888 · 2024-03-01T13:31:00Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffiterm.hpp

    static FFITerm Const(const std::string& val, Solver* slv, uint32_t base = 16);

+    explicit FFITerm(bb::fr value, Solver* s){
+        std::stringstream buf; // TODO(alex): looks bad. Would be great to create tostring() converter


Please add an issue

Rumata888 · 2024-03-01T13:34:19Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffiterm.test.cpp

+
+using namespace smt_terms;
+
+TEST(integermod, addition)


could you change the name of the class of tests from integermod to FFITerm?

Rumata888 · 2024-03-01T13:35:36Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffterm.hpp

    static FFTerm Const(const std::string& val, Solver* slv, uint32_t base = 16);

+    FFTerm(bb::fr value, Solver* s){
+        std::stringstream buf; // TODO(alex): looks bad. Would be great to create tostring() converter


Please create an issue

Rumata888 · 2024-03-01T13:38:08Z

barretenberg/cpp/src/barretenberg/smt_verification/terms/ffterm.test.cpp

+
+using namespace smt_terms;
+
+TEST(finitefield, addition)


Could you rename from finitefield to FFTerm?

- changed division method to be more readable - changed the hash value that is used as a dvision symbolic variable name to md5(Aztec) - Removed the mod operations from inequalities methods in FFITerm - Modified `==` and `!=` methods in FFITerm so now there won't be any redundant mod operations

Rumata888

LGTM

- FFTerm -> FFITerm

Merging to merge

AztecBot · 2024-03-05T20:24:13Z

Benchmark results

No metrics with a significant change found.

Detailed results

All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.

This benchmark source data is available in JSON format on S3 here.

Values are compared against data from master at commit e899e56e and shown if the difference exceeds 1%.

L2 block published to L1

Each column represents the number of txs on an L2 block published to L1.

Metric	8 txs	32 txs	64 txs
l1_rollup_calldata_size_in_bytes	5,700	18,884	36,452
l1_rollup_calldata_gas	66,108	238,904	469,712
l1_rollup_execution_gas	194,056	500,210	908,970
l2_block_processing_time_in_ms	1,200 (-1%)	4,512	8,997
note_successful_decrypting_time_in_ms	205 (+1%)	554 (-1%)	1,013 (-3%)
note_trial_decrypting_time_in_ms	9.39 (-12%)	72.9 (-12%)	50.9 (-7%)
l2_block_building_time_in_ms	16,383 (+1%)	64,752	130,045
l2_block_rollup_simulation_time_in_ms	12,446 (+1%)	49,217	98,941
l2_block_public_tx_process_time_in_ms	3,907 (+1%)	15,465	30,939

L2 chain processing

Each column represents the number of blocks on the L2 chain where each block has 16 txs.

Metric	5 blocks	10 blocks
node_history_sync_time_in_ms	14,353	26,951 (+1%)
note_history_successful_decrypting_time_in_ms	1,254	2,313 (-4%)
note_history_trial_decrypting_time_in_ms	106 (+17%)	111 (-20%)
node_database_size_in_bytes	18,767,952	35,590,224
pxe_database_size_in_bytes	29,923	59,478

Circuits stats

Stats on running time and I/O sizes collected for every circuit run across all benchmarks.

Circuit	circuit_simulation_time_in_ms	circuit_input_size_in_bytes	circuit_output_size_in_bytes
private-kernel-init	252	44,736	28,001
private-kernel-ordering	193	52,625	14,627
base-rollup	1,303 (-1%)	177,932	933
root-rollup	70.1	4,192	825
private-kernel-inner	318 (-1%)	73,715	28,001
public-kernel-app-logic	193 (-2%)	32,254	25,379
merge-rollup	5.70 (-2%)	2,712	933

Tree insertion stats

The duration to insert a fixed batch of leaves into each tree type.

Metric	1 leaves	2 leaves	8 leaves	16 leaves	32 leaves	64 leaves	128 leaves	512 leaves	1024 leaves	2048 leaves	4096 leaves
batch_insert_into_append_only_tree_16_depth_ms	9.80 (-1%)	10.4 (-2%)	16.9	16.0 (-2%)	22.1 (-3%)	35.6 (+1%)	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_16_depth_hash_count	16.9	17.5	23.0	31.6	47.0	79.0	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_16_depth_hash_ms	0.568 (-1%)	0.578 (-2%)	0.715	0.495 (-2%)	0.461 (-2%)	0.442 (+1%)	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_32_depth_ms	N/A	N/A	N/A	N/A	N/A	45.7 (-1%)	71.4 (-2%)	231	435 (-2%)	873	1,733 (+1%)
batch_insert_into_append_only_tree_32_depth_hash_count	N/A	N/A	N/A	N/A	N/A	96.0	159	543	1,055	2,079	4,127
batch_insert_into_append_only_tree_32_depth_hash_ms	N/A	N/A	N/A	N/A	N/A	0.468 (-1%)	0.440 (-1%)	0.421	0.407 (-2%)	0.415	0.415
batch_insert_into_indexed_tree_20_depth_ms	N/A	N/A	N/A	N/A	N/A	55.0 (+2%)	106 (-2%)	340 (-1%)	644 (-3%)	1,319	2,623
batch_insert_into_indexed_tree_20_depth_hash_count	N/A	N/A	N/A	N/A	N/A	104	207	691	1,363	2,707	5,395
batch_insert_into_indexed_tree_20_depth_hash_ms	N/A	N/A	N/A	N/A	N/A	0.489 (+2%)	0.478 (-1%)	0.458	0.444 (-3%)	0.457	0.456
batch_insert_into_indexed_tree_40_depth_ms	N/A	N/A	N/A	N/A	60.5 (-1%)	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_indexed_tree_40_depth_hash_count	N/A	N/A	N/A	N/A	109	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_indexed_tree_40_depth_hash_ms	N/A	N/A	N/A	N/A	0.529 (-1%)	N/A	N/A	N/A	N/A	N/A	N/A

Miscellaneous

Transaction sizes based on how many contracts are deployed in the tx.

Metric	0 deployed contracts
tx_size_in_bytes	19,179

Transaction processing duration by data writes.

Metric	0 new note hashes	1 new note hashes
tx_pxe_processing_time_ms	2,582 (-2%)	1,372

Metric	0 public data writes	1 public data writes
tx_sequencer_processing_time_ms	0.0330 (-1%)	472 (-1%)

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.26.1</summary> ## [0.26.1](aztec-package-v0.26.0...aztec-package-v0.26.1) (2024-03-06) ### Miscellaneous * **aztec-package:** Synchronize aztec-packages versions </details> <details><summary>barretenberg.js: 0.26.1</summary> ## [0.26.1](barretenberg.js-v0.26.0...barretenberg.js-v0.26.1) (2024-03-06) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-cli: 0.26.1</summary> ## [0.26.1](aztec-cli-v0.26.0...aztec-cli-v0.26.1) (2024-03-06) ### Miscellaneous * **aztec-cli:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.26.1</summary> ## [0.26.1](aztec-packages-v0.26.0...aztec-packages-v0.26.1) (2024-03-06) ### Features * Adding fr compatibility to smt variables api ([#4884](#4884)) ([c085cbb](c085cbb)) * **avm-simulator:** Implement EMITUNENCRYPTEDLOG ([#4926](#4926)) ([5f3304e](5f3304e)) * Choose constructor method in Contract.deploy ([#4939](#4939)) ([e899e56](e899e56)) * Indirect mem flag deserialisation ([#4877](#4877)) ([4c6820f](4c6820f)) ### Miscellaneous * Add missing jobs to CI end ([#4963](#4963)) ([ff4110e](ff4110e)) * **avm-simulator:** Better type env getters ([#4950](#4950)) ([8f97977](8f97977)) * **avm-simulator:** Revive field comparison ([#4957](#4957)) ([ee21374](ee21374)) * **avm-simulator:** Test improvements ([#4946](#4946)) ([f74e6a1](f74e6a1)) * Fix CCI config ([#4974](#4974)) ([40178f0](40178f0)) * Remove commitment key copy out of instance ([#4893](#4893)) ([6eb6778](6eb6778)) * **vscode:** Add avm-transpiler to vscode rust-analyzer settings ([#4952](#4952)) ([db915e5](db915e5)) </details> <details><summary>barretenberg: 0.26.1</summary> ## [0.26.1](barretenberg-v0.26.0...barretenberg-v0.26.1) (2024-03-06) ### Features * Adding fr compatibility to smt variables api ([#4884](#4884)) ([c085cbb](c085cbb)) * Indirect mem flag deserialisation ([#4877](#4877)) ([4c6820f](4c6820f)) ### Miscellaneous * Remove commitment key copy out of instance ([#4893](#4893)) ([6eb6778](6eb6778)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.26.1</summary> ## [0.26.1](AztecProtocol/aztec-packages@aztec-package-v0.26.0...aztec-package-v0.26.1) (2024-03-06) ### Miscellaneous * **aztec-package:** Synchronize aztec-packages versions </details> <details><summary>barretenberg.js: 0.26.1</summary> ## [0.26.1](AztecProtocol/aztec-packages@barretenberg.js-v0.26.0...barretenberg.js-v0.26.1) (2024-03-06) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-cli: 0.26.1</summary> ## [0.26.1](AztecProtocol/aztec-packages@aztec-cli-v0.26.0...aztec-cli-v0.26.1) (2024-03-06) ### Miscellaneous * **aztec-cli:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.26.1</summary> ## [0.26.1](AztecProtocol/aztec-packages@aztec-packages-v0.26.0...aztec-packages-v0.26.1) (2024-03-06) ### Features * Adding fr compatibility to smt variables api ([#4884](AztecProtocol/aztec-packages#4884)) ([c085cbb](AztecProtocol/aztec-packages@c085cbb)) * **avm-simulator:** Implement EMITUNENCRYPTEDLOG ([#4926](AztecProtocol/aztec-packages#4926)) ([5f3304e](AztecProtocol/aztec-packages@5f3304e)) * Choose constructor method in Contract.deploy ([#4939](AztecProtocol/aztec-packages#4939)) ([e899e56](AztecProtocol/aztec-packages@e899e56)) * Indirect mem flag deserialisation ([#4877](AztecProtocol/aztec-packages#4877)) ([4c6820f](AztecProtocol/aztec-packages@4c6820f)) ### Miscellaneous * Add missing jobs to CI end ([#4963](AztecProtocol/aztec-packages#4963)) ([ff4110e](AztecProtocol/aztec-packages@ff4110e)) * **avm-simulator:** Better type env getters ([#4950](AztecProtocol/aztec-packages#4950)) ([8f97977](AztecProtocol/aztec-packages@8f97977)) * **avm-simulator:** Revive field comparison ([#4957](AztecProtocol/aztec-packages#4957)) ([ee21374](AztecProtocol/aztec-packages@ee21374)) * **avm-simulator:** Test improvements ([#4946](AztecProtocol/aztec-packages#4946)) ([f74e6a1](AztecProtocol/aztec-packages@f74e6a1)) * Fix CCI config ([#4974](AztecProtocol/aztec-packages#4974)) ([40178f0](AztecProtocol/aztec-packages@40178f0)) * Remove commitment key copy out of instance ([#4893](AztecProtocol/aztec-packages#4893)) ([6eb6778](AztecProtocol/aztec-packages@6eb6778)) * **vscode:** Add avm-transpiler to vscode rust-analyzer settings ([#4952](AztecProtocol/aztec-packages#4952)) ([db915e5](AztecProtocol/aztec-packages@db915e5)) </details> <details><summary>barretenberg: 0.26.1</summary> ## [0.26.1](AztecProtocol/aztec-packages@barretenberg-v0.26.0...barretenberg-v0.26.1) (2024-03-06) ### Features * Adding fr compatibility to smt variables api ([#4884](AztecProtocol/aztec-packages#4884)) ([c085cbb](AztecProtocol/aztec-packages@c085cbb)) * Indirect mem flag deserialisation ([#4877](AztecProtocol/aztec-packages#4877)) ([4c6820f](AztecProtocol/aztec-packages@4c6820f)) ### Miscellaneous * Remove commitment key copy out of instance ([#4893](AztecProtocol/aztec-packages#4893)) ([6eb6778](AztecProtocol/aztec-packages@6eb6778)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Sarkoxed added 4 commits March 1, 2024 11:15

New more detailed CMakeLists

b266440

Modified FFTerm and FFITerm classes

fe27688

- Now they are compatible with Fr variables - Changed FFIterm constant initialization

Added tests for symbolic variables classes

5b3a4dc

Removed xor test from ffiterm leaving only arithmetic + fr include

07987cf

Sarkoxed requested a review from Rumata888 March 1, 2024 12:52

Rumata888 reviewed Mar 1, 2024

View reviewed changes

Sarkoxed added 3 commits March 4, 2024 22:27

Fix todos to be linked to issue

dca609a

Renamed the test classes

20397aa

Rumata888 marked this pull request as ready for review March 5, 2024 15:32

Rumata888 approved these changes Mar 5, 2024

View reviewed changes

Sarkoxed and others added 6 commits March 5, 2024 15:35

Merge branch 'master' into as/smt-fr-compatiblity

37ade6c

Merge branch 'master' into as/smt-fr-compatiblity

511f5ce

Fixing a typo in readme.md

cc53f79

- FFTerm -> FFITerm

Merge branch 'master' into as/smt-fr-compatiblity

874d30c

Merging to merge

Merge branch 'master' into as/smt-fr-compatiblity

41cac90

Merge branch 'master' into as/smt-fr-compatiblity

311296e

Sarkoxed self-assigned this Mar 5, 2024

Rumata888 added crypto cryptography product-security PRs extending our security mechanisms labels Mar 5, 2024

Rumata888 added 2 commits March 5, 2024 19:51

Merge branch 'master' into as/smt-fr-compatiblity

bb49dce

formatting fix

d46e267

Sarkoxed merged commit c085cbb into master Mar 5, 2024

Sarkoxed deleted the as/smt-fr-compatiblity branch March 5, 2024 21:01

AztecBot mentioned this pull request Mar 5, 2024

chore(master): Release 0.26.1 #4964

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adding fr compatibility to smt variables api#4884

feat: Adding fr compatibility to smt variables api#4884
Sarkoxed merged 15 commits intomasterfrom
as/smt-fr-compatiblity

Sarkoxed commented Mar 1, 2024 •

edited

Loading

Uh oh!

Rumata888 Mar 1, 2024 •

edited

Loading

Uh oh!

Rumata888 Mar 1, 2024

Uh oh!

Rumata888 Mar 1, 2024

Uh oh!

Rumata888 Mar 1, 2024

Uh oh!

Rumata888 Mar 1, 2024

Uh oh!

Rumata888 Mar 1, 2024

Uh oh!

Rumata888 Mar 1, 2024

Uh oh!

Rumata888 left a comment

Uh oh!

AztecBot commented Mar 5, 2024

L2 block published to L1

L2 chain processing

Circuits stats

Tree insertion stats

Miscellaneous

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Sarkoxed commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

FFTerm

FFITerm

Tests

updt

Terms

Uh oh!

Rumata888 Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rumata888 Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

Rumata888 Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

Rumata888 Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

Rumata888 Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

Rumata888 Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

Rumata888 Mar 1, 2024

Choose a reason for hiding this comment

Uh oh!

Rumata888 left a comment

Choose a reason for hiding this comment

Uh oh!

AztecBot commented Mar 5, 2024

Benchmark results

L2 block published to L1

L2 chain processing

Circuits stats

Tree insertion stats

Miscellaneous

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sarkoxed commented Mar 1, 2024 •

edited

Loading

Rumata888 Mar 1, 2024 •

edited

Loading