e2e test for a public token transfer#325
Conversation
benesjan
force-pushed
the
janb/e2e_public_token
branch
3 times, most recently
from
17c5d71 to
d4c7774
Compare
benesjan
force-pushed
the
janb/e2e_public_token
branch
from
abe7659 to
b83a2d9
Compare
This reverts commit e74315e.
…c3-packages into janb/e2e_public_token
|
|
||
| describe('mint', () => { | ||
| it('should run the mint function', async () => { | ||
| it.only('should run the mint function', async () => { |
| // Retry fetching from the storage slot until it's non-zero | ||
| const fn = async () => { | ||
| const storageSlot = await calculateStorageSlot(accountIdx); | ||
| console.log('storage slot', storageSlot.toString()); |
There was a problem hiding this comment.
Remove the console logs. If we want to log we should use the logger object.
| this.sequencer.restart(); | ||
| } | ||
|
|
||
| public getStateTransitions() { |
There was a problem hiding this comment.
I don't think we want this.
| const privateDataTreeInfo = await this.db.getTreeInfo(MerkleTreeId.PRIVATE_DATA_TREE); | ||
| const nullifierTreeInfo = await this.db.getTreeInfo(MerkleTreeId.NULLIFIER_TREE); | ||
|
|
||
| publicKernelOutput.constants.historicTreeRoots.privateHistoricTreeRoots.nullifierTreeRoot = Fr.fromBuffer( |
There was a problem hiding this comment.
@spalladino we had to add these as there is a validation step in the circuit block builder that failed otherwise. It looks like there is a bit of a hack in aztec node to supply these values for private functions.
There was a problem hiding this comment.
Heh, that's exactly what I pushed yesterday for private functions, see #425
There was a problem hiding this comment.
So I think we should not be adding these manually to the kernel circuit output, but rather we should make sure they are available as part of the inputs, and the circuit should be forwarding them to the outputs.
I think the problem is that the PublicKernelInputsNoPreviousKernel, having no previous kernel, has no access to the constants, so it never receives the tree roots as input and cannot forward them to the outputs (as the private kernel circuit does here).
The fix would be to modify the PublicKernelInputsNoPreviousKernel so it also receives a CombinedConstantData which is used to populate the outputs. However, if we're removing this circuit, maybe we can just live with this hack a bit longer.
WDYT @PhilWindle?
There was a problem hiding this comment.
I think the plan is to modify the private kernel circuit so it also has a 'no previous kernel' mode which would result in the same issue. A question for @iAmMichaelConnor really. In the case of 'no previous kernel' iterations, should the constant data (tree roots) be supplied as an input to the circuit so the circuit can forward it to the circuit's public inputs? Currently the caller of the circuit (aztec rpc or sequencer) has to enrich this data after the 'no previous kernel' iteration.
|
|
||
| // Pad arrays to reach required length | ||
| const result = await execution.run(); | ||
| const args = tx.args; |
There was a problem hiding this comment.
@sirasistant we had an issue running the acvm that it was complaining of arrays not being padded. So for example the args array only had 3 items instead of 8. I couldn't find if/where you were padding the arrays in the private client.
| const isSynced = await accountState?.isSynchronised(); | ||
| if (accountState && !isSynced) { |
There was a problem hiding this comment.
This change causes isSynchronised to be queried always, which involves a roundtrip to the node. Should we revert this?
| if (storageValue === undefined) { | ||
| throw new Error(`Storage slot ${storageSlot} not found`); | ||
| } | ||
|
|
There was a problem hiding this comment.
| if (storageValue === undefined) { | |
| throw new Error(`Storage slot ${storageSlot} not found`); | |
| } |
Duplicated
| args.push(...new Array<Fr>(ARGS_LENGTH - tx.args.length).fill(Fr.ZERO)); | ||
|
|
||
| const { stateReads, stateTransitions, returnValues } = result; | ||
| returnValues.push(...new Array<Fr>(RETURN_VALUES_LENGTH - result.returnValues.length).fill(Fr.ZERO)); |
There was a problem hiding this comment.
We need a nicer pad function :-P
spypsy
force-pushed
the
janb/e2e_public_token
branch
from
ba89fcc to
c9ca30f
Compare
4 participants
Description
Fixes #265
Checklist: