Valid contract address should be known in constructor #179
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Valid contract address should be known in constructor and should be input to private kernel even for contract deployment.
Slack conversation summarized:
"""
Alvaro Rodriguez
During constructor execution, call_context.storage_contract_address should be the precomputed contract address right? not zero
(in the noir context)
I was exposing zero and changed it to the precomputed contract address in the acir simulator and everything seems to work
But i don't know how the real private kernel will handle that
Mike
Hmm... well actually...
At the moment, the private kernel computes the new contract address and will then hash any new_commitments with the new contract address.
Edit: it also enforces that the value is 0 in the case of a constructor, so that would need to change.
Does a constructor function need to 'know' about its own contract address?
Can you access address(this) inside a Solidity constructor?
Alvaro Rodriguez
I think that the noir code is also siloing commitments 😕 we should remove that
But if i remember correctly i think that the constructor in solidity can know its own address. It also can call other contracts that use msg.sender, for example
Mike
The Noir code should silo the commitment when it does a read (get). But not when it does a write (insert).
Alvaro Rodriguez
Yeah, on read you need it for merkle membership, but it was siloing on write
https://github.com/AztecProtocol/aztec3-packages/blob/master/yarn-project/noir-contracts/src/contracts/noir-aztec3/src/set.nr#L77
Mike
Ah, well spotted, please could you update that when you get a chance?
Alvaro Rodriguez
yep!
Mike
But if i remember correctly i think that the constructor in solidity can know its own address
I just checked and you can. So we should update the call_context to always have a nonzero storage_contract_address.
@dbanks12 this might be a nice thing for you to try to update in the Private Kernel Circuit logic. At the moment we enforce that the storage_contract_address is zero (in the case of executing a constructor), but we should instead check that this value matches the contract address which we derive. (Should be a very tiny change).
Leila
I think we can still continue to have the to field of the TxRequest be 0 (in the case of contract deployment). So we shouldn't need a change there.
TxRequest.to is always the contract address, for both regular tx and contract deployment.
Mike
Oh, cool
@dbanks12 please can you also check that the kernel circuit isn't enforcing a 0 in the to field of the TxRequest 😅
"""
Checklist:
/specs
have been updated.@brief
describing the intended functionality.