Skip to content

feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member#13971

Merged
dbanks12 merged 1 commit intomasterfrom
db/getcontractinstance-dst
May 1, 2025
Merged

feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member#13971
dbanks12 merged 1 commit intomasterfrom
db/getcontractinstance-dst

Conversation

@dbanks12
Copy link
Contributor

@dbanks12 dbanks12 commented Apr 30, 2025

This is easier to constrain in the AVM as otherwise it is the only opcode with two destination offset operands.

@dbanks12 dbanks12 marked this pull request as ready for review April 30, 2025 18:53
Copy link
Contributor Author

dbanks12 commented Apr 30, 2025

@dbanks12 dbanks12 requested review from IlyasRidhuan and removed request for fcarreiro April 30, 2025 18:54
Comment on lines -40 to +57
pub unconstrained fn get_contract_instance_deployer_internal_avm(
unconstrained fn get_contract_instance_deployer_internal_avm(
address: AztecAddress,
) -> (Field, bool) {
) -> [GetContractInstanceResult; 1] {
get_contract_instance_deployer_oracle_avm(address)
}
pub unconstrained fn get_contract_instance_class_id_internal_avm(
unconstrained fn get_contract_instance_class_id_internal_avm(
address: AztecAddress,
) -> (Field, bool) {
) -> [GetContractInstanceResult; 1] {
get_contract_instance_class_id_oracle_avm(address)
}
pub unconstrained fn get_contract_instance_initialization_hash_internal_avm(
unconstrained fn get_contract_instance_initialization_hash_internal_avm(
address: AztecAddress,
) -> (Field, bool) {
) -> [GetContractInstanceResult; 1] {
Copy link
Contributor Author

@dbanks12 dbanks12 Apr 30, 2025

Choose a reason for hiding this comment

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

We need create a struct inside an array of length 1 because we need this to compile to a brillig opcode with one destination HeapArray where the first entry is bool and second is field. If we just use a struct, it'll compile to a brillig opcode with two destination MemoryAddresses.

Also, not sure why these were pub before since they're internal. Is it okay to remove pub here? @benesjan

Copy link
Contributor

Choose a reason for hiding this comment

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

Ser, you wrote the code 😆

But agree that pub shouldn't be there.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol yes I know! 🤣 I had to hover many lines before I found your name in the file. But since you're an "aztec nr code owner" so to say, I thought I'd make sure this is okay!

@dbanks12 dbanks12 changed the title feat: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member May 1, 2025
@dbanks12 dbanks12 changed the title feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member [WIP] feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member May 1, 2025
@dbanks12 dbanks12 force-pushed the db/getcontractinstance-dst branch from 53169db to cb68e71 Compare May 1, 2025 15:44
…set gets exists and dstOffset+1 gets instance member
@dbanks12 dbanks12 force-pushed the db/getcontractinstance-dst branch from cb68e71 to 16b4193 Compare May 1, 2025 16:28
@dbanks12 dbanks12 requested review from Maddiaa0 and jeanmon as code owners May 1, 2025 16:28
@dbanks12 dbanks12 changed the title [WIP] feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member feat!: GETCONTRACTINSTANCE opcode has 1 dstOffset operand where dstOffset gets exists and dstOffset+1 gets instance member May 1, 2025
Copy link
Contributor

@IlyasRidhuan IlyasRidhuan left a comment

Choose a reason for hiding this comment

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

LGTM

// Setters for inputs and output for gadgets/subtraces. These are used for register allocation.
void set_inputs(std::vector<TaggedValue> inputs) { this->inputs = std::move(inputs); }
void set_outputs(std::vector<TaggedValue> outputs) { this->outputs = std::move(outputs); }
void set_output(TaggedValue output) { this->output = std::move(output); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@dbanks12 dbanks12 enabled auto-merge May 1, 2025 17:14
@dbanks12 dbanks12 added this pull request to the merge queue May 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 1, 2025
@dbanks12 dbanks12 added this pull request to the merge queue May 1, 2025
Merged via the queue into master with commit e4ee6e9 May 1, 2025
8 of 9 checks passed
@dbanks12 dbanks12 deleted the db/getcontractinstance-dst branch May 1, 2025 18:56
IlyasRidhuan added a commit that referenced this pull request Mar 13, 2026
`sel_op_dc_17` was unused since #13971 and can be safely removed.
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.

3 participants