fix: make bytecode part of artifact hash preimage again (#9771)

In this PR, we are simply trying to re-introduce the bytecode hash as a
part of the preimage to the function artifact hash.

I had tried to reproduce the weird noir non-deterministic bytecode bug
that @spalladino documented
[here](https://aztecprotocol.slack.com/archives/C053490AV6V/p1713480846092319?thread_ts=1713445232.979779&cid=C053490AV6V)
referenced in #5860 but was unable to do so.

I see only matched bytecode hashes on local and CI:
<img width="807" alt="•noir-projects+build-contracts bar_far,"
src="https://github.com/user-attachments/assets/404ae02f-4f72-4ea4-a5e1-43b2d96d507b">

<img width="733" alt="Pasted Graphic 6"
src="https://github.com/user-attachments/assets/1a7e15bf-6ca6-45ab-bfa7-4b97ea336e61">
 

If this has been already fixed by the noir team it may make sense that
this should be merged in to simply re-add the security check.

Author: sklppy88

yarn-project/circuits.js/src/contract/artifact_hash.test.ts

@@ -30,7 +30,7 @@ describe('ArtifactHash', () => {
 
     for (let i = 0; i < 1000; i++) {
       expect(computeArtifactHash(testArtifact).toString()).toMatchInlineSnapshot(
-        `"0x237feccc8e34a39c0e5133c8653fc278b39275bfa3f7459e4aba07d53b752c19"`,
+        `"0x21070d88558fdc3906322f267cf6f0f632caf3949295520fe1f71f156fbb0d0b"`,
       );
     }
   });
@@ -43,7 +43,7 @@ describe('ArtifactHash', () => {
     const testArtifact = loadContractArtifact(content);
 
     expect(computeArtifactHash(testArtifact).toString()).toMatchInlineSnapshot(
-      `"0x237feccc8e34a39c0e5133c8653fc278b39275bfa3f7459e4aba07d53b752c19"`,
+      `"0x21070d88558fdc3906322f267cf6f0f632caf3949295520fe1f71f156fbb0d0b"`,
     );
   });
 });

yarn-project/circuits.js/src/contract/artifact_hash.ts

@@ -92,11 +92,10 @@ export function computeFunctionArtifactHash(
     | (Pick<FunctionArtifact, 'bytecode'> & { functionMetadataHash: Fr; selector: FunctionSelector }),
 ) {
   const selector = 'selector' in fn ? fn.selector : FunctionSelector.fromNameAndParameters(fn);
-  // TODO(#5860): make bytecode part of artifact hash preimage again
-  // const bytecodeHash = sha256Fr(fn.bytecode).toBuffer();
-  // const metadataHash = 'functionMetadataHash' in fn ? fn.functionMetadataHash : computeFunctionMetadataHash(fn);
-  // return sha256Fr(Buffer.concat([numToUInt8(VERSION), selector.toBuffer(), metadataHash.toBuffer(), bytecodeHash]));
-  return sha256Fr(Buffer.concat([numToUInt8(VERSION), selector.toBuffer()]));
+
+  const bytecodeHash = sha256Fr(fn.bytecode).toBuffer();
+  const metadataHash = 'functionMetadataHash' in fn ? fn.functionMetadataHash : computeFunctionMetadataHash(fn);
+  return sha256Fr(Buffer.concat([numToUInt8(VERSION), selector.toBuffer(), metadataHash.toBuffer(), bytecodeHash]));
 }
 
 export function computeFunctionMetadataHash(fn: FunctionArtifact) {

yarn-project/circuits.js/src/contract/private_function_membership_proof.test.ts

@@ -31,8 +31,7 @@ describe('private_function_membership_proof', () => {
     expect(isValidPrivateFunctionMembershipProof(fn, contractClass)).toBeTruthy();
   });
 
-  // TODO(#5860): Re-enable this test once noir non-determinism is addressed
-  test.skip.each([
+  test.each([
     'artifactTreeSiblingPath',
     'artifactMetadataHash',
     'functionMetadataHash',

yarn-project/circuits.js/src/contract/unconstrained_function_membership_proof.test.ts

@@ -50,8 +50,7 @@ describe('unconstrained_function_membership_proof', () => {
     expect(isValidUnconstrainedFunctionMembershipProof(fn, contractClass)).toBeTruthy();
   });
 
-  // TODO(#5860): Re-enable this test once noir non-determinism is addressed
-  test.skip.each(['artifactTreeSiblingPath', 'artifactMetadataHash', 'functionMetadataHash'] as const)(
+  test.each(['artifactTreeSiblingPath', 'artifactMetadataHash', 'functionMetadataHash'] as const)(
     'fails proof if %s is mangled',
     field => {
       const proof = createUnconstrainedFunctionMembershipProof(selector, artifact);

yarn-project/pxe/src/pxe_service/pxe_service.ts

@@ -250,10 +250,7 @@ export class PXEService implements PXE {
           `Artifact does not match expected class id (computed ${contractClassId} but instance refers to ${instance.contractClassId})`,
         );
       }
-      if (
-        // Computed address from the instance does not match address inside instance
-        !computeContractAddressFromInstance(instance).equals(instance.address)
-      ) {
+      if (!computeContractAddressFromInstance(instance).equals(instance.address)) {
         throw new Error('Added a contract in which the address does not match the contract instance.');
       }