feat(Authwit): lookup the validity of authwits

[LHerskind]

Fixes #4822.

The current solution is pretty ugly (I think).

The issues encountered is mainly that:

• There are no context so we need to pass stuff that seems quite redundant (address of the contract itself and block number)
• Fetching witness data with oracles explodes if you try to fetch something that does not exists.
• The implementation here is a lookup from "outside" a transaction.

The latter is the main trouble. It can be somewhat mitigated by having a function on the typescript side of things that try doing the lookup in its local storage first to not encounter that case, but then we need to be able to split the check so seems a bit meh to fix it that way.

This means that the unconstrained cannot really be used nicely as what @spalladino did at the offsite, but mainly is an off-chain thing for the user who is unsure if something was approved and want to check it.

I'm not really sure if I like to have the implementation this way where we are doing things in both ts and noir to get it somewhat working, but seems like the thing that is making it the easiest to get a query going fast.

[LHerskind]

• feat(AuthWit): chain_id and version in hash #5331
• feat(Authwit): lookup the validity of authwits #5316 👈
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @LHerskind and the rest of your teammates on Graphite

[sklppy88]

Oh goodness that is nasty, calling own account contract from within ts 😝.

Super cool though and I don't see why this doesn't work for now.

[AztecBot]

Benchmark results

No metrics with a significant change found.

Detailed results

All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.

This benchmark source data is available in JSON format on S3 here.

Values are compared against data from master at commit fec10081 and shown if the difference exceeds 1%.

L2 block published to L1

Each column represents the number of txs on an L2 block published to L1.

Metric	8 txs	32 txs	64 txs
l1_rollup_calldata_size_in_bytes	5,668	18,852	36,420
l1_rollup_calldata_gas	66,268	239,232	470,016
l1_rollup_execution_gas	680,961	951,469	1,312,542
l2_block_processing_time_in_ms	1,299 (+2%)	4,517 (-4%)	9,359
note_successful_decrypting_time_in_ms	178 (-7%)	512	955 (+1%)
note_trial_decrypting_time_in_ms	85.8 (-11%)	29.1 (-5%)	86.3 (+6%)
l2_block_building_time_in_ms	17,795	66,785 (-1%)	132,444
l2_block_rollup_simulation_time_in_ms	8,005	28,497	56,018
l2_block_public_tx_process_time_in_ms	9,769 (+1%)	38,236 (-1%)	76,330 (-1%)

L2 chain processing

Each column represents the number of blocks on the L2 chain where each block has 16 txs.

Metric	5 blocks	10 blocks
node_history_sync_time_in_ms	14,385 (+2%)	26,570 (-4%)
note_history_successful_decrypting_time_in_ms	1,214	2,462
note_history_trial_decrypting_time_in_ms	68.0 (+12%)	174 (+12%)
node_database_size_in_bytes	18,665,552	35,106,896
pxe_database_size_in_bytes	29,859	59,414

Circuits stats

Stats on running time and I/O sizes collected for every circuit run across all benchmarks.

Circuit	circuit_simulation_time_in_ms	circuit_input_size_in_bytes	circuit_output_size_in_bytes
private-kernel-init	278 (-1%)	44,366	28,245
private-kernel-ordering	213	52,869	14,327
base-parity	1,779 (-2%)	128	311
base-rollup	710	165,788	925
root-parity	1,584 (+2%)	1,244	311
root-rollup	51.9 (+1%)	4,487	725
private-kernel-inner	639 (-1%)	73,772	28,245
public-kernel-app-logic	437 (+3%)	35,260	28,215
public-kernel-tail	175 (+2%)	40,926	28,215
merge-rollup	8.39 (+1%)	2,696	925

Tree insertion stats

The duration to insert a fixed batch of leaves into each tree type.

Metric	1 leaves	16 leaves	64 leaves	128 leaves	512 leaves	1024 leaves	2048 leaves	4096 leaves	32 leaves
batch_insert_into_append_only_tree_16_depth_ms	9.97	16.0	N/A	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_16_depth_hash_count	16.8	31.6	N/A	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_16_depth_hash_ms	0.582	0.496	N/A	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_32_depth_ms	N/A	N/A	45.8	72.0	230	445	875 (+1%)	1,721	N/A
batch_insert_into_append_only_tree_32_depth_hash_count	N/A	N/A	96.0	159	543	1,055	2,079	4,127	N/A
batch_insert_into_append_only_tree_32_depth_hash_ms	N/A	N/A	0.470	0.445	0.420	0.416	0.416 (+1%)	0.413	N/A
batch_insert_into_indexed_tree_20_depth_ms	N/A	N/A	54.8 (+2%)	107 (+1%)	337 (+1%)	672 (+2%)	1,312 (+1%)	2,610	N/A
batch_insert_into_indexed_tree_20_depth_hash_count	N/A	N/A	104	207	691	1,363	2,707	5,395	N/A
batch_insert_into_indexed_tree_20_depth_hash_ms	N/A	N/A	0.489 (+2%)	0.481 (+1%)	0.458	0.463 (+2%)	0.455 (+1%)	0.455 (+1%)	N/A
batch_insert_into_indexed_tree_40_depth_ms	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	60.9
batch_insert_into_indexed_tree_40_depth_hash_count	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	109
batch_insert_into_indexed_tree_40_depth_hash_ms	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	0.534

Miscellaneous

Transaction sizes based on how many contract classes are registered in the tx.

Metric	0 registered classes
tx_size_in_bytes	22,013

Transaction processing duration by data writes.

Metric	0 new note hashes	1 new note hashes
tx_pxe_processing_time_ms	3,213 (-1%)	1,735 (-1%)

Metric	0 public data writes	1 public data writes
tx_sequencer_processing_time_ms	11.0 (-16%)	1,226 (+2%)

[spalladino]

Approving because the code definitely works, but I agree in that the solution feels somewhat ugly, especially as the verify_signature call gets repeated in the unconstrained version.

Maybe we should break down the different uses for this, to try and nail down the best way to implement it? In my mind there are four scenarios: querying from within a tx (the one we faced at the offsite and motivated #4822) or from offchain (the one addressed in this PR), both for private and public land.

Within a tx for private authwits, we want to ask the local PXE if there's a valid authwit for an action, in order to consume it immediately afterwards. The interaction would probably be something like:

let action = compute_authwit_hash(...)
if check_has_valid_authwit(sender, action):
  assert_valid_authwit(sender, action)
else:
  try_something_else()

Here check_has_valid_authwit could share code with assert_valid_authwit in that it loads the authwit from the PXE and validates it, but not spends it. The nullifier check for cancellations is probably not needed in the check_has_valid_authwit, since the PXE can know when an authwit has been consumed and thus delete it from its cache. So we could have something like:

contract Account:
  internal private check_signature(authwit):
    // This is the only custom code for each account contract
    return schnorr_check(authwit, this.pubkey)

  private check_has_valid_authwit(action):
    // Oracle should return an empty authwit if it has been spent
    let authwit = oracle.get_authwit(EXPECTED_AUTHWIT_SIZE_FOR_THIS_ACCOUNT_CONTRACT)
    if authwit.is_empty(): return false
    return check_signature(authwit)

  private assert_valid_authwit(action):
    assert(check_has_valid_authwit(action))
    emit_nullifier(action)

Within a tx for public authwits should be easier, since authwits are set in storage. Right now I understand that public authwits also emit a nullifier from public with consumed, but if we also removed it from storage, we could just read public state and return that as the check.

Offchain for private authwits feels like another instance of a recurring problem we have: duplicating getters for onchain and offchain. I think we should tackle that bigger issue rather than start adding more duplicated getters. If we find a way to call a private function from off-chain (ie just simulating it, which is something we sort-of can already do today), we could just use check_valid_authwit and be done with it.

Offchain for public authwits should also be a matter of simulating a public function to read storage, in line with the one above.

The one thing I'm not sure about here is the naming for check_has_valid_authwit, since it should be clear that it's unsafe: it depends on the PXE being willing to return the authwit, as well as being honest and not returning it if it's spent. But if we can convey in the name that it should always be followed by an assert_valid_authwit, then I think this should do the trick. WDYT?

[spalladino]

Why do we need a full membership witness if we're blinding trusting the oracle? We could just return a flag "spent". Or is this just to avoid introducing a new oracle call?

[LHerskind]

Was to not introduce a new oracle 👍

[nventuro]

This means that the unconstrained cannot really be used nicely as what @spalladino did at the offsite, but mainly is an off-chain thing for the user who is unsure if something was approved and want to check it.

Are you refering to having to e.g. inject the address because we're not in a transaction context? If so I'd say the deeper issue is in this execution mode being so different from the other two, ideally we'd be able to make this as close as possible to an actual tx.

[nventuro]

This would be solved by #2665 right?

[LHerskind]

Not directly as this one is doing both public and private, likely better addressed by having a unconstrained context as we briefly discussed the other day in the scrum. Should likely have a larger issue as it can also align functions so they are not totally different.

[nventuro]

Why do we need to add this manually? Also this is missing the check_private param. How is this working with that value missing?

[LHerskind]

Was just done as such to be similar to the others. Interesting to see that it seems to be somewhat ignored that stuff is missing in the unconstrained 🤣 My guess would almost be that it is going straight for Fields if nothing specified then. But quite odd.

[LHerskind]

Interestingly, if you are viewing it is not using the encodeArguments function but just blindly passing the arguments along so it does not use anything beyond the name of the function it seems

  public view(options: ViewMethodOptions = {}) {
    if (this.functionDao.functionType !== FunctionType.UNCONSTRAINED) {
      throw new Error('Can only call `view` on an unconstrained function.');
    }

    const { from } = options;
    return this.wallet.viewTx(this.functionDao.name, this.args, this.contractAddress, from);
  }

You can entirely delete the parameters and returnTypes from the abi and it will still succeed if unconstrained.

[nventuro]

What's the todo? docs?

[LHerskind]

Yes 😬