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: Public Kernel must check contract membership #1200

Open
1 of 6 tasks
LHerskind opened this issue Jul 26, 2023 · 1 comment
Open
1 of 6 tasks

feat: Public Kernel must check contract membership #1200

LHerskind opened this issue Jul 26, 2023 · 1 comment

Comments

@LHerskind
Copy link
Contributor

LHerskind commented Jul 26, 2023

Following the introduction of the internal keyword, the kernels must enforce that the contract itself is the caller for internal functions. This is handled for private calls in #978.

For public calls is was realised that there was no contract membership check. The current PublicCallData does not include membership witnesses, vk nor the acir_hash, so would need to be extended to include those, or clarify how the function leaf is setup.

Secondly, when doing public calls, the executor currently does not have access to the function abi, so it will not be able to provide the needed information for this check or the is_internal without extensions.

Tasks

Preview Give feedback
  1. C-avm
    IlyasRidhuan
  2. LHerskind

Example, bypassing the is_internal flag for executing public functions.

If we insert false as is_internal here, we can bypass the check that is enforced by the circuits, as it enforce the check based on the inputs you give it and currently don't check the contract matches was you provided.

const functionData = new FunctionData(
generateFunctionSelector(this.functionDao.name, this.functionDao.parameters),
this.functionDao.isInternal,
this.functionDao.functionType === FunctionType.SECRET,
this.functionDao.name === 'constructor',
);


Indifferent tree

When computing DeploymentInfo with getContractDeploymentInfo() we compute the contract leaves as follows

const vkHash = hashVKStr(f.verificationKey!, wasm);
// TODO
// FIXME: https://github.com/AztecProtocol/aztec3-packages/issues/262
// const acirHash = keccak(Buffer.from(f.bytecode, 'hex'));
const acirHash = Buffer.alloc(32, 0);

This does not make a distinction between private and public function calls, and will compute public as if they were private function calls. For public calls, the acir_hash should be 0, and instead of the vk_hash we should use the opcodes_commitment which from the cpp seem to be referred to as bytecode_hash.

This will have an effect on how we are to perform the membership checks so is important to update.

Computing the bytecode hash

As mentioned in #378 it is not decided how the bytecode hash is represented.

@LHerskind LHerskind changed the title feat: Public Kernel must enforce feat: Public Kernel must enforce is_internal Jul 26, 2023
@LHerskind LHerskind added this to A3 Jul 31, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Jul 31, 2023
@LHerskind LHerskind self-assigned this Jul 31, 2023
@LHerskind LHerskind changed the title feat: Public Kernel must enforce is_internal feat: Public Kernel must check contract membership Jul 31, 2023
@LHerskind LHerskind moved this from Todo to Blocked in A3 Aug 1, 2023
@LHerskind LHerskind removed their assignment Oct 11, 2023
@LHerskind
Copy link
Contributor Author

@dbanks12 moving this issue to your team. Seems like a better fit since you are dealing with the kernels more than I, and need to add related fixed anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Blocked
Development

No branches or pull requests

1 participant