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

feat(avm): trace contract class and contract instance #8840

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

IlyasRidhuan
Copy link
Contributor

@IlyasRidhuan IlyasRidhuan commented Sep 26, 2024

This PR is centred around tracing and passing contract class & instance during simulator execution and passing it to circuit. We store each contract class & instance whenever the simulator calls getBytecode.

This changes the input interface to the bb binary - we no longer take in a specific bytecode to execute. Instead we get a vector of {contract_class, contract_instance, bytecode} which define all the (deduplicated) contract bytecode that will be executed during this "one-enqueued call" (actual implementation of 1-enqueued call tbd).

This doesnt do any derivation of id or address yet

@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch 2 times, most recently from 2fabc9f to 595f109 Compare September 27, 2024 13:31
@IlyasRidhuan IlyasRidhuan changed the title feat(avm): class id + contract address feat(avm): trace contract class and contract instance Sep 27, 2024
, contract_instance(contract_instance)
, contract_class_id_preimage(contract_class_id_preimage)
{}
AvmContractBytecode(std::vector<uint8_t> bytecode)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have this constructor because some of the execution tests just use bytecode

@IlyasRidhuan IlyasRidhuan marked this pull request as ready for review September 27, 2024 13:46
constructor(
public readonly bytecode: Buffer,
public contractInstanceHint: AvmContractInstanceHint,
public contractClassHint: ContractClassIdPreimage,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are definitely "over-hinting" since we also transmit the address, bytecode commitment and class id.

But for now these are useful in checking that the values calculated in witgen match the simulator

Copy link
Contributor

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

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

Nice work! I have a few comments, but nothing that couldn't be handled in a follow-up PR.

exists = false;
contractInstance = SerializableContractInstance.empty().withAddress(contractAddress);
this.trace.traceExecutionStart(
bytecode ?? Buffer.alloc(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do Buffer.alloc(0) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I'm not really sure what the behavior should be if we can't find the bytecode - do we throw (because we can't execute the instructions in the simulator anyways)?

If not we should hint some representation of "empty". I think the circuit can't put nothing in the column because then it wouldn't be provable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we decided that if you call a contract that doesn't exist, the call instruction should just do nothing and return success=0 with empty returndata.

So do whatever's easiest for the bytecode hashing, because then we'll just throw out the results and prove that the contract address passed to the call instruction doesn't exist in the nullifier tree.

yarn-project/simulator/src/avm/journal/journal.ts Outdated Show resolved Hide resolved
yarn-project/simulator/src/public/side_effect_trace.ts Outdated Show resolved Hide resolved
yarn-project/simulator/src/public/side_effect_trace.ts Outdated Show resolved Hide resolved
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-10-feat_bytecode_hashing_init branch from c093e9f to f7e7f7a Compare September 30, 2024 22:28
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch from 595f109 to fee5e1b Compare October 1, 2024 00:13
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-10-feat_bytecode_hashing_init branch from f7e7f7a to 81dd3de Compare October 1, 2024 09:01
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch from fee5e1b to b22807c Compare October 1, 2024 09:01
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-10-feat_bytecode_hashing_init branch from 81dd3de to 769d979 Compare October 1, 2024 13:05
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch from b22807c to 32228e8 Compare October 1, 2024 13:06
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-10-feat_bytecode_hashing_init branch 3 times, most recently from 6f0f85a to b3882fc Compare October 18, 2024 13:25
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch from 32228e8 to 5686e86 Compare October 18, 2024 16:44
Copy link
Contributor

github-actions bot commented Oct 18, 2024

Changes to public function bytecode sizes

Generated at commit: a23cc9511f509863513a21b37341b841c4ce89da, compared to commit: 7766c8e714185d6e8b9fa392d7f371fb30da8f1a

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
AvmTest::public_dispatch -713 ✅ -1.32%
AvmTest::bulk_testing -1,355 ✅ -5.72%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
AvmTest::public_dispatch 53,252 (-713) -1.32%
AvmTest::bulk_testing 22,336 (-1,355) -5.72%

@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch 2 times, most recently from 7743702 to 18309e2 Compare October 21, 2024 07:32
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-10-feat_bytecode_hashing_init branch from b3882fc to 8850b0f Compare October 22, 2024 15:51
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-10-feat_bytecode_hashing_init branch 5 times, most recently from 265bb38 to 5b44970 Compare October 25, 2024 11:48
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch from 18309e2 to a05eb6d Compare October 25, 2024 12:39
Base automatically changed from ir/09-10-feat_bytecode_hashing_init to master October 25, 2024 14:13
@IlyasRidhuan IlyasRidhuan force-pushed the ir/09-25-feat_trace_contract_remove_bytecode branch from a05eb6d to 689fdbf Compare October 25, 2024 14:17
@IlyasRidhuan IlyasRidhuan merged commit 84205d8 into master Oct 25, 2024
62 checks passed
@IlyasRidhuan IlyasRidhuan deleted the ir/09-25-feat_trace_contract_remove_bytecode branch October 25, 2024 14:53
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