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.

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

fix: smt_verification: negative bitvecs, changed gates indicies. acir_formal_proofs: noir-style signed division #11649

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

jewelofchaos9
Copy link
Contributor

Negative bitvector values didnt parse properly

@jewelofchaos9 jewelofchaos9 changed the title fix: Fix negative values in BVTerms of smt_solver fix: smt_verification: negative bitvecs, changed gates indicies. acir_formal_proofs: noir-style signed division Feb 3, 2025
@@ -30,18 +30,18 @@ UltraCircuit::UltraCircuit(
// add gate in its normal state to solver

size_t arith_cursor = 0;
while (arith_cursor < this->selectors[1].size()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add an entry to ultra_circuit.hpp with the changed indices

Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding the pow2_8, you should use bit_extraction rather then direct & 1 value

smt_circuit::STerm shl64(smt_circuit::STerm v0, smt_circuit::STerm v1, smt_solver::Solver* solver)
{
auto shifted = shl(v0, v1, solver);
// 2^64 - 1
auto mask = smt_terms::BVConst("18446744073709551615", solver, 10);
auto res = shifted & mask;
/*auto mask = smt_terms::BVConst("18446744073709551615", solver, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

are these comments necessary?

@Sarkoxed Sarkoxed added the product-security PRs extending our security mechanisms label Feb 5, 2025

TEST(helpers, signed_div_2)
{
// using smt solver i found that 1879048194 >> 16 == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you find out why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was bug in my implementation

@@ -89,4 +89,87 @@ TEST(helpers, pow2)
info("z = ", vals["z"]);
// z == 2048 in binary
EXPECT_TRUE(vals["z"] == "00000000000000000000100000000000");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Leave a few comments what are these tests doing

@@ -429,6 +429,35 @@ STerm STerm::rotl(const uint32_t& n) const
return { res, this->solver, this->type };
}

STerm STerm::truncate(const uint32_t& to_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add a test and update README.md with these new methods?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product-security PRs extending our security mechanisms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants