Skip to content

chore: stdlib field pre-audit pt.0#14413

Merged
iakovenkos merged 9 commits intomasterfrom
si/looking-at-field
May 22, 2025
Merged

chore: stdlib field pre-audit pt.0#14413
iakovenkos merged 9 commits intomasterfrom
si/looking-at-field

Conversation

@iakovenkos
Copy link
Contributor

@iakovenkos iakovenkos commented May 20, 2025

Minor docs and test changes, improve readability by replacing nested ternary operators with a helper, get rid of 2 small unused methods.

@iakovenkos iakovenkos self-assigned this May 20, 2025

static field_t coset_generator(const size_t generator_idx)
{
return field_t(bb::fr::coset_generator(generator_idx));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these two methods are not needed, since our verifiers don't use FFT

// if a < b, this implies b - a - 1 < K
// if a >= b, this implies b - a + K - 1 < K
// i.e. (b - a - 1) * q + (b - a + K - 1) * (1 - q) = r < K
// Let q = (a < b)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly expanded the explanation here

template <typename Builder> field_t<Builder> field_t<Builder>::madd(const field_t& to_mul, const field_t& to_add) const
{
Builder* ctx = (context == nullptr) ? (to_mul.context == nullptr ? to_add.context : to_mul.context) : context;
Builder* ctx = first_non_null<Builder>(context, to_mul.context, to_add.context);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here and below, use first_non_null to avoid nested ternary operators


if ((to_mul.witness_index == IS_CONSTANT) && (to_add.witness_index == IS_CONSTANT) &&
(witness_index == IS_CONSTANT)) {
if (to_mul.is_constant() && to_add.is_constant() && this->is_constant()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here and below, replace foo.witness_index == IS_CONSTANT with foo.is_constant() that performs the same check, but seems more readable/expressive

ASSERT(this->multiplicative_constant == bb::fr::one());
// A constant field_t's value is tracked wholly by its additive_constant member.
return additive_constant;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed redundant else

template <typename Builder>
std::vector<bool_t<Builder>> field_t<Builder>::decompose_into_bits(
const size_t num_bits, const std::function<witness_t<Builder>(Builder*, uint64_t, uint256_t)> get_bit) const
const std::function<witness_t<Builder>(Builder*, uint64_t, uint256_t)> get_bit) const
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the num_bits parameter wasn't handled correctly and the only non-test appearance used the default value 256, so I removed the argument.
moreover, this method seems like a good candidate to be deleted

static void test_constructor_from_witness()
{
bb::fr val = 2;
bb::fr val = fr::random_element();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some randomness wouldn't hurt


/**
* @brief Test that conditional assign doesn't produce a new witness
* @brief Test that conditional assign doesn't produce a new witness if lhs and rhs are constant
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description was a bit misleading. Expanded the test to all possible options for the predicate - constant/witness true/false

@@ -951,20 +964,20 @@ template <typename Builder> class stdlib_field : public testing::Test {

static void test_add_two()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test was disabled, just re-enabled it

@iakovenkos iakovenkos changed the title draft: stdlib field explorations draft: stdlib field pre-audit pt.0 May 22, 2025
@iakovenkos iakovenkos marked this pull request as ready for review May 22, 2025 10:09
@iakovenkos iakovenkos changed the title draft: stdlib field pre-audit pt.0 chore: stdlib field pre-audit pt.0 May 22, 2025
@iakovenkos iakovenkos requested a review from ledwards2225 May 22, 2025 12:08
Copy link
Contributor

@ledwards2225 ledwards2225 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@iakovenkos iakovenkos added this pull request to the merge queue May 22, 2025
Merged via the queue into master with commit 52458c2 May 22, 2025
4 checks passed
@iakovenkos iakovenkos deleted the si/looking-at-field branch May 22, 2025 18:08
benesjan pushed a commit that referenced this pull request May 23, 2025
Minor docs and test changes, improve readability by replacing nested
ternary operators with a helper, get rid of 2 small unused methods.
Thunkar pushed a commit that referenced this pull request May 23, 2025
Minor docs and test changes, improve readability by replacing nested
ternary operators with a helper, get rid of 2 small unused methods.
charlielye pushed a commit that referenced this pull request May 28, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.87.3](v0.87.2...v0.87.3)
(2025-05-28)


### ⚠ BREAKING CHANGES

* app benches and unified PXE creation
([#14504](#14504))
* Removes normalize() calls on pairing points
([#14285](#14285))

### Features

* Adds StarknetZK WASM bindings to bb.js
([#14372](#14372))
([224b219](224b219))
* app benches and unified PXE creation
([#14504](#14504))
([aae5ab0](aae5ab0))
* **bb:** memory tracking for microbenchmarks
([#14445](#14445))
([fae2961](fae2961))
* enable provers to run in node chart
([#14405](#14405))
([94edd35](94edd35))
* measure oracles
([#14552](#14552))
([9cc6b54](9cc6b54))
* Removes normalize() calls on pairing points
([#14285](#14285))
([942b948](942b948))
* unbundled bb.js
([#14401](#14401))
([e0d9662](e0d9662))
* validating partial note sender
([#14379](#14379))
([de9880c](de9880c)),
closes
[#14363](#14363)


### Bug Fixes

* asan-fast caching
([#14468](#14468))
([fbd9ed6](fbd9ed6))
* attempt fix concurrency
([f31e706](f31e706))
* attempt to fix merge group base config
([6aa342b](6aa342b))
* bug in `TxProvingResult` schema
([#14498](#14498))
([#14530](#14530))
([0ac26fa](0ac26fa))
* bump defaults
([#14474](#14474))
([ade9a56](ade9a56))
* **docs:** Update getting started with testnet page
([#14536](#14536))
([66fc3eb](66fc3eb))
* don't create ./out when verifying
([#14556](#14556))
([ccb9981](ccb9981))
* eccvm_circuit_builder overrun
([#14484](#14484))
([5e01c07](5e01c07))
* issues with syncNotes --&gt; syncPrivateState renaming
([#14442](#14442))
([78de410](78de410))
* Merge queue instances have pr name for uniquness in e.g. reorgs.
([#14562](#14562))
([1f012a9](1f012a9))
* nope
([470882a](470882a))
* **playground:** fix sfpc version when creating account
([#14481](#14481))
([5486a22](5486a22))
* processing events in contracts with no notes
([#14528](#14528))
([1bab9b4](1bab9b4)),
closes
[#14499](#14499)
* slack for flakes
([61829bc](61829bc))
* target branch
([e463723](e463723))
* test tracking
([#14513](#14513))
([2d6fc3a](2d6fc3a))
* try to fix release please
([f31c07e](f31c07e))
* try to fix release please
([d402c27](d402c27))
* try to fix release please
([cb959b8](cb959b8))
* try to fix release please
([227dbd2](227dbd2))
* update sponsored fpc address in playground
([#14472](#14472))
([4c14bc9](4c14bc9))
* wip merge master to next
([249ab4c](249ab4c))
* wip merge master to next
([b7850c9](b7850c9))


### Miscellaneous

* **bb:** avoid compile-time hash-to-curve
([#14177](#14177))
([d3863ff](d3863ff))
* **docs:** Update alpha-testnet docs to use version 0.87.2
([#14501](#14501))
([c86d3b0](c86d3b0))
* master-to-next
([#14454](#14454))
([06ac335](06ac335))
* **master:** release 0.87.3
([#14440](#14440))
([3ca26cd](3ca26cd))
* partial notes optimization with nullifier
([#14432](#14432))
([cc90823](cc90823))
* playground should release also at refname. attempt to make RP do
prerelease version.
([3c4fba5](3c4fba5))
* Protocol breaking changes must now go to next branch.
([#14423](#14423))
([44304b6](44304b6))
* release 0.87.3
([8340415](8340415))
* release please v4
([bf97805](bf97805))
* remove some expected failures
([#13843](#13843))
([2593e28](2593e28))
* select random rollup versions from distinct sets
([#14568](#14568))
([fe787cd](fe787cd))
* stdlib field pre-audit pt.0
([#14413](#14413))
([52458c2](52458c2))


### Documentation

* comment on partial note reuse
([#14383](#14383))
([42200c8](42200c8))
* fix image
([#14537](#14537))
([388e79b](388e79b)),
closes
[#14163](#14163)
* update node version instructions
([#14412](#14412))
([a2b1b9d](a2b1b9d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
charlielye pushed a commit that referenced this pull request Jun 2, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.87.6](v0.87.5...v0.87.6)
(2025-06-02)


### ⚠ BREAKING CHANGES

* app benches and unified PXE creation
([#14504](#14504))
* Removes normalize() calls on pairing points
([#14285](#14285))

### Features

* Adds StarknetZK WASM bindings to bb.js
([#14372](#14372))
([224b219](224b219))
* allow mempool to overflow before evicting
([#14641](#14641))
([f81425d](f81425d))
* app benches and unified PXE creation
([#14504](#14504))
([aae5ab0](aae5ab0))
* **bb:** memory tracking for microbenchmarks
([#14445](#14445))
([fae2961](fae2961))
* enable provers to run in node chart
([#14405](#14405))
([94edd35](94edd35))
* Ignore P2P messages previously seen
([#14665](#14665))
([2e50588](2e50588))
* measure oracles
([#14552](#14552))
([9cc6b54](9cc6b54))
* Removes normalize() calls on pairing points
([#14285](#14285))
([942b948](942b948))
* Request missing txs from block proposal sender
([#14341](#14341))
([3f97e8b](3f97e8b))
* unbundled bb.js
([#14401](#14401))
([e0d9662](e0d9662))
* validate notes in a single node roundtrip
([#14650](#14650))
([d467955](d467955))
* validating partial note sender
([#14379](#14379))
([de9880c](de9880c)),
closes
[#14363](#14363)


### Bug Fixes

* allow returning of tuples from contract funcs
([#14553](#14553))
([1c0955c](1c0955c))
* asan-fast caching
([#14468](#14468))
([fbd9ed6](fbd9ed6))
* attempt fix concurrency
([f31e706](f31e706))
* attempt to fix merge group base config
([6aa342b](6aa342b))
* bug in `TxProvingResult` schema
([#14498](#14498))
([#14530](#14530))
([0ac26fa](0ac26fa))
* bump defaults
([#14474](#14474))
([ade9a56](ade9a56))
* do not run concurrent idb transactions
([#14609](#14609))
([5a6f36a](5a6f36a))
* **docs:** Update create pxe interface
([#14587](#14587))
([b3b9c05](b3b9c05))
* **docs:** Update getting started with testnet page
([#14536](#14536))
([66fc3eb](66fc3eb))
* don't create ./out when verifying
([#14556](#14556))
([ccb9981](ccb9981))
* Don't manually close stream on p2p/reqresp/goodbye
([#14531](#14531))
([38cec7d](38cec7d))
* eccvm_circuit_builder overrun
([#14484](#14484))
([5e01c07](5e01c07))
* enable bundling txs with block proposals
([#14649](#14649))
([39b8c4e](39b8c4e))
* error on goodbye
([#14679](#14679))
([5112f5f](5112f5f))
* issues with syncNotes --&gt; syncPrivateState renaming
([#14442](#14442))
([78de410](78de410))
* Merge queue instances have pr name for uniquness in e.g. reorgs.
([#14562](#14562))
([1f012a9](1f012a9))
* nope
([470882a](470882a))
* parse LOG_LEVELS in bb
([#14674](#14674))
([88203d6](88203d6))
* **playground:** fix sfpc version when creating account
([#14481](#14481))
([5486a22](5486a22))
* prevent world-state from spamming the logs
([#14594](#14594))
([8c725de](8c725de))
* processing events in contracts with no notes
([#14528](#14528))
([1bab9b4](1bab9b4)),
closes
[#14499](#14499)
* retrieve L1 to L2 messages in batches
([#14586](#14586))
([2be7f1b](2be7f1b))
* slack for flakes
([61829bc](61829bc))
* target branch
([e463723](e463723))
* test tracking
([#14513](#14513))
([2d6fc3a](2d6fc3a))
* try to fix release please
([f31c07e](f31c07e))
* try to fix release please
([d402c27](d402c27))
* try to fix release please
([cb959b8](cb959b8))
* try to fix release please
([227dbd2](227dbd2))
* update sponsored fpc address in playground
([#14472](#14472))
([4c14bc9](4c14bc9))
* wip merge master to next
([249ab4c](249ab4c))
* wip merge master to next
([b7850c9](b7850c9))


### Miscellaneous

* add testnet compat test
([#14601](#14601))
([1fbb350](1fbb350))
* Aztec simulator into PXE
([#14598](#14598))
([a999c8c](a999c8c))
* **bb:** avoid compile-time hash-to-curve
([#14177](#14177))
([d3863ff](d3863ff))
* capture RPC calls + correct timings
([#14633](#14633))
([5ee54ff](5ee54ff))
* Disable flood publish by default
([#14635](#14635))
([af8d879](af8d879))
* **docs:** Add scheduled typesense index job
([#14615](#14615))
([5f42e1b](5f42e1b))
* **docs:** Update alpha-testnet docs to use version 0.87.2
([#14501](#14501))
([c86d3b0](c86d3b0))
* drop txs per block to 8
([#14627](#14627))
([369f210](369f210))
* master-to-next
([#14454](#14454))
([06ac335](06ac335))
* **master:** release 0.87.3
([#14440](#14440))
([3ca26cd](3ca26cd))
* **master:** release 0.87.3
([#14582](#14582))
([8cea842](8cea842))
* **master:** release 0.87.4
([#14583](#14583))
([8077f63](8077f63))
* New prover chart for alpha-testnet
([#14514](#14514))
([031d7cb](031d7cb))
* partial notes optimization with nullifier
([#14432](#14432))
([cc90823](cc90823))
* playground should release also at refname. attempt to make RP do
prerelease version.
([3c4fba5](3c4fba5))
* Protocol breaking changes must now go to next branch.
([#14423](#14423))
([44304b6](44304b6))
* release 0.87.3
([8340415](8340415))
* release please v4
([bf97805](bf97805))
* remove some expected failures
([#13843](#13843))
([2593e28](2593e28))
* Revert "fix: processing events in contracts with no notes
([#14528](#14528))"
([#14596](#14596))
([3278e61](3278e61))
* select random rollup versions from distinct sets
([#14568](#14568))
([fe787cd](fe787cd))
* stdlib field pre-audit pt.0
([#14413](#14413))
([52458c2](52458c2))
* trick release-please
([cbcad3d](cbcad3d))
* trick release-please
([a0c3a21](a0c3a21))


### Documentation

* comment on partial note reuse
([#14383](#14383))
([42200c8](42200c8))
* create v0.87.4 as latest
([#14607](#14607))
([78c2469](78c2469))
* fix image
([#14537](#14537))
([388e79b](388e79b)),
closes
[#14163](#14163)
* update cli ref in node doc
([#14574](#14574))
([d54a8af](d54a8af))
* update node version instructions
([#14412](#14412))
([a2b1b9d](a2b1b9d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants