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

Update selector computation #1990

Closed
Tracked by #1743
LHerskind opened this issue Sep 4, 2023 · 0 comments · Fixed by #2001
Closed
Tracked by #1743

Update selector computation #1990

LHerskind opened this issue Sep 4, 2023 · 0 comments · Fixed by #2001
Assignees
Labels
A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-bug Type: Bug. Something is broken.

Comments

@LHerskind
Copy link
Contributor

LHerskind commented Sep 4, 2023

The current function selector computation is not ideal.
It is weird that it is using field while the noir code is using Field and if using a struct, the function signature which is used for the selector just inserts struct meaning that two functions with the same name that have structs as input will have same selector even if different structs.

Function Signature: transfer(struct,struct,field)
Function Selector:  0x6f9c917c

Similar if using arrays. In the function below, every param is different types, but you don't know from the signature or selector

#[aztec(private)]
fn test_selector(
    _struct: AztecAddress,
    _other_struct: EthereumAddress,
    _field: Field,
    _u8: u8,
    _array: [Field; 2],
    _u8_array: [u8; 2],
) {}

Function Signature: test_selector(struct,struct,field,integer,array,array)
Function Selector:  0x1b5ca441

The function signature should be unambiguous and look closer to the noir code that is represents.


Proposed solution

I propose that we add struct as tuples, that integers also have their width, and that arrays are inserted similarly to their definitions with sizes. An example is seen below:

struct Asset {
    interest_accumulator: u120,
    last_updated_ts: u120,
    loan_to_value: u120,
    oracle_address: Field,
}

struct Position {
    collateral: Field,
    static_debt: Field,
    debt: Field,
}

struct Wrapped {
    asset: Asset,
    position: Position,
    array: [Field; 2],
}

#[aztec(private)]
fn abi_tester(
    asset: Asset,
    wrapped: Wrapped,
    owner: Field,
    small: u8,
    array: [Field; 2],
    u8array: [u8; 2],
    u120array: [u120; 2],
) {}

// Current
// Function Signature: abi_tester(struct,struct,field,integer,array,array,array)
// Function Selector:  0xd1b771b8

// My proposal
// Function Signature: abi_tester((u120,u120,u120,Field),((u120,u120,u120,Field),(Field,Field,Field),[Field;2]),Field,u8,[Field;2],[u8;2],[u120;2])
// Function Selector:  0xaec244be
@LHerskind LHerskind added this to the 📢 Initial Public Sandbox Release milestone Sep 4, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Sep 4, 2023
@LHerskind LHerskind added T-bug Type: Bug. Something is broken. A-internal-devex Area: Relates to the devex of internal teams building Aztec. labels Sep 4, 2023
@LHerskind LHerskind self-assigned this Sep 5, 2023
@LHerskind LHerskind moved this from Todo to In Progress in A3 Sep 5, 2023
@LHerskind LHerskind moved this from In Progress to Blocked in A3 Sep 5, 2023
@LHerskind LHerskind moved this from Blocked to In Review in A3 Sep 5, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in A3 Sep 5, 2023
LHerskind added a commit that referenced this issue Sep 5, 2023
Fixed #1990. Also adds an oracle to more easily compute selectors from
inside the contracts for improved devex.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-bug Type: Bug. Something is broken.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant