Skip to content

Commit

Permalink
chore!: remove keccak256 opcode from ACIR/Brillig (#9104)
Browse files Browse the repository at this point in the history
This PR removes the keccak256 opcode as we never emit this now,
preferring keccakf1600. As we have #8989 making a breaking change to
serialisation, this is a good time to do this to avoid an extra
serialisation change.
  • Loading branch information
TomAFrench authored Oct 11, 2024
1 parent 3a01ad9 commit 4c1163a
Show file tree
Hide file tree
Showing 43 changed files with 17 additions and 697 deletions.
11 changes: 0 additions & 11 deletions avm-transpiler/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 0 additions & 23 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1040,29 +1040,6 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
..Default::default()
});
}
BlackBoxOp::Keccak256 { message, output } => {
let message_offset = message.pointer.to_usize();
let message_size_offset = message.size.to_usize();
let dest_offset = output.pointer.to_usize();
assert_eq!(output.size, 32, "Keccak256 output size must be 32!");

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::KECCAK,
indirect: Some(
AddressingModeBuilder::default()
.indirect_operand(&output.pointer)
.indirect_operand(&message.pointer)
.direct_operand(&message.size)
.build(),
),
operands: vec![
AvmOperand::U32 { value: dest_offset as u32 },
AvmOperand::U32 { value: message_offset as u32 },
AvmOperand::U32 { value: message_size_offset as u32 },
],
..Default::default()
});
}
BlackBoxOp::Keccakf1600 { message, output } => {
let message_offset = message.pointer.to_usize();
let message_size_offset = message.size.to_usize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,7 @@ void build_constraints(Builder& builder,
constraint_system.original_opcode_indices.blake3_constraints.at(i));
}

// Add keccak constraints
for (size_t i = 0; i < constraint_system.keccak_constraints.size(); ++i) {
const auto& constraint = constraint_system.keccak_constraints.at(i);
create_keccak_constraints(builder, constraint);
gate_counter.track_diff(constraint_system.gates_per_opcode,
constraint_system.original_opcode_indices.keccak_constraints.at(i));
}

// Add keccak permutations
for (size_t i = 0; i < constraint_system.keccak_permutations.size(); ++i) {
const auto& constraint = constraint_system.keccak_permutations[i];
create_keccak_permutations(builder, constraint);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ struct AcirFormatOriginalOpcodeIndices {
std::vector<size_t> ecdsa_r1_constraints;
std::vector<size_t> blake2s_constraints;
std::vector<size_t> blake3_constraints;
std::vector<size_t> keccak_constraints;
std::vector<size_t> keccak_permutations;
std::vector<size_t> pedersen_constraints;
std::vector<size_t> pedersen_hash_constraints;
Expand Down Expand Up @@ -95,7 +94,6 @@ struct AcirFormat {
std::vector<EcdsaSecp256r1Constraint> ecdsa_r1_constraints;
std::vector<Blake2sConstraint> blake2s_constraints;
std::vector<Blake3Constraint> blake3_constraints;
std::vector<KeccakConstraint> keccak_constraints;
std::vector<Keccakf1600> keccak_permutations;
std::vector<PedersenConstraint> pedersen_constraints;
std::vector<PedersenHashConstraint> pedersen_hash_constraints;
Expand Down Expand Up @@ -147,7 +145,6 @@ struct AcirFormat {
ecdsa_r1_constraints,
blake2s_constraints,
blake3_constraints,
keccak_constraints,
keccak_permutations,
pedersen_constraints,
pedersen_hash_constraints,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ TEST_F(AcirFormatTests, TestASingleConstraintNoPubInputs)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -175,7 +174,6 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -258,7 +256,6 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -367,7 +364,6 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -430,98 +426,6 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange)
EXPECT_EQ(verifier.verify_proof(proof), true);
}

TEST_F(AcirFormatTests, TestVarKeccak)
{
HashInput input1;
input1.witness = 0;
input1.num_bits = 8;
HashInput input2;
input2.witness = 1;
input2.num_bits = 8;
HashInput input3;
input3.witness = 2;
input3.num_bits = 8;
KeccakConstraint keccak;
keccak.inputs = { input1, input2, input3 };
keccak.var_message_size = 3;
keccak.result = { 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35 };

RangeConstraint range_a{
.witness = 0,
.num_bits = 8,
};
RangeConstraint range_b{
.witness = 1,
.num_bits = 8,
};
RangeConstraint range_c{
.witness = 2,
.num_bits = 8,
};
RangeConstraint range_d{
.witness = 3,
.num_bits = 8,
};

auto dummy = poly_triple{
.a = 0,
.b = 0,
.c = 0,
.q_m = 0,
.q_l = 1,
.q_r = 0,
.q_o = 0,
.q_c = fr::neg_one() * fr(4),
};

AcirFormat constraint_system{
.varnum = 36,
.recursive = false,
.num_acir_opcodes = 6,
.public_inputs = {},
.logic_constraints = {},
.range_constraints = { range_a, range_b, range_c, range_d },
.aes128_constraints = {},
.sha256_compression = {},
.schnorr_constraints = {},
.ecdsa_k1_constraints = {},
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = { keccak },
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
.poseidon2_constraints = {},
.multi_scalar_mul_constraints = {},
.ec_add_constraints = {},
.recursion_constraints = {},
.honk_recursion_constraints = {},
.avm_recursion_constraints = {},
.ivc_recursion_constraints = {},
.bigint_from_le_bytes_constraints = {},
.bigint_to_le_bytes_constraints = {},
.bigint_operations = {},
.assert_equalities = {},
.poly_triple_constraints = { dummy },
.quad_constraints = {},
.big_quad_constraints = {},
.block_constraints = {},
.original_opcode_indices = create_empty_original_opcode_indices(),
};
mock_opcode_indices(constraint_system);

WitnessVector witness{ 4, 2, 6, 2 };
auto builder = create_circuit(constraint_system, /*size_hint*/ 0, witness);

auto composer = Composer();
auto prover = composer.create_ultra_with_keccak_prover(builder);
auto proof = prover.construct_proof();
auto verifier = composer.create_ultra_with_keccak_verifier(builder);
EXPECT_EQ(verifier.verify_proof(proof), true);
}

TEST_F(AcirFormatTests, TestKeccakPermutation)
{
Keccakf1600
Expand Down Expand Up @@ -571,7 +475,6 @@ TEST_F(AcirFormatTests, TestKeccakPermutation)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = { keccak_permutation },
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -648,7 +551,6 @@ TEST_F(AcirFormatTests, TestCollectsGateCounts)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -777,7 +679,6 @@ TEST_F(AcirFormatTests, TestBigAdd)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ acir_format::AcirFormatOriginalOpcodeIndices create_empty_original_opcode_indice
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -63,9 +62,6 @@ void mock_opcode_indices(acir_format::AcirFormat& constraint_system)
for (size_t i = 0; i < constraint_system.blake3_constraints.size(); i++) {
constraint_system.original_opcode_indices.blake3_constraints.push_back(current_opcode++);
}
for (size_t i = 0; i < constraint_system.keccak_constraints.size(); i++) {
constraint_system.original_opcode_indices.keccak_constraints.push_back(current_opcode++);
}
for (size_t i = 0; i < constraint_system.keccak_permutations.size(); i++) {
constraint_system.original_opcode_indices.keccak_permutations.push_back(current_opcode++);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -647,24 +647,6 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg,
af.constrained_witness.insert(af.ec_add_constraints.back().result_y);
af.constrained_witness.insert(af.ec_add_constraints.back().result_infinite);
af.original_opcode_indices.ec_add_constraints.push_back(opcode_index);
} else if constexpr (std::is_same_v<T, Program::BlackBoxFuncCall::Keccak256>) {
auto input_var_message_size = get_witness_from_function_input(arg.var_message_size);
af.keccak_constraints.push_back(KeccakConstraint{
.inputs = map(arg.inputs,
[](auto& e) {
auto input_witness = get_witness_from_function_input(e);
return HashInput{
.witness = input_witness,
.num_bits = e.num_bits,
};
}),
.result = map(arg.outputs, [](auto& e) { return e.value; }),
.var_message_size = input_var_message_size,
});
for (auto& output : af.keccak_constraints.back().result) {
af.constrained_witness.insert(output);
}
af.original_opcode_indices.keccak_constraints.push_back(opcode_index);
} else if constexpr (std::is_same_v<T, Program::BlackBoxFuncCall::Keccakf1600>) {
af.keccak_permutations.push_back(Keccakf1600{
.state = map(arg.inputs, [](auto& e) { return parse_input(e); }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ TEST_F(BigIntTests, TestBigIntConstraintMultiple)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -260,7 +259,6 @@ TEST_F(BigIntTests, TestBigIntConstraintSimple)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -320,7 +318,6 @@ TEST_F(BigIntTests, TestBigIntConstraintReuse)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -385,7 +382,6 @@ TEST_F(BigIntTests, TestBigIntConstraintReuse2)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -471,7 +467,6 @@ TEST_F(BigIntTests, TestBigIntDIV)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ TEST_F(UltraPlonkRAM, TestBlockConstraint)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -206,7 +205,6 @@ TEST_F(MegaHonk, Databus)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -315,7 +313,6 @@ TEST_F(MegaHonk, DatabusReturn)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ TEST_F(EcOperations, TestECOperations)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -213,7 +212,6 @@ TEST_F(EcOperations, TestECMultiScalarMul)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintSucceed)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -163,7 +162,6 @@ TEST_F(ECDSASecp256k1, TestECDSACompilesForVerifier)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down Expand Up @@ -215,7 +213,6 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintFail)
.ecdsa_r1_constraints = {},
.blake2s_constraints = {},
.blake3_constraints = {},
.keccak_constraints = {},
.keccak_permutations = {},
.pedersen_constraints = {},
.pedersen_hash_constraints = {},
Expand Down
Loading

1 comment on commit 4c1163a

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 4c1163a Previous: 3a01ad9 Ratio
Goblin::merge(t) 164602531 ns/iter 151137904 ns/iter 1.09

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.