-
Notifications
You must be signed in to change notification settings - Fork 271
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
feat: non-hardcoded constants #7736
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
mod lib; | ||
|
||
contract PrivateFPC { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just cleaned up imports here. Unrelated to constants change |
||
use dep::aztec::protocol_types::{abis::log_hash::LogHash, address::AztecAddress, hash::poseidon2_hash}; | ||
use dep::aztec::state_vars::SharedImmutable; | ||
use dep::aztec::{protocol_types::{address::AztecAddress, hash::poseidon2_hash}, state_vars::SharedImmutable}; | ||
use dep::token_with_refunds::TokenWithRefunds; | ||
use crate::lib::emit_randomness_as_unencrypted_log; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,9 +68,8 @@ unconstrained fn setup_refund_success() { | |
utils::check_private_balance(token_contract_address, fee_payer, 1) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated this comment since it was imprecise. Unrelated to constants change |
||
// TODO(#7694): Ideally we would check the error message here but it's currently not possible because TXE does not | ||
// support checking messages of errors thrown in a public teardown function. Once this is supported, check the message | ||
// here and delete the e2e test checking it. | ||
// TODO(#7694): Ideally we would check the error message here but it's currently not supported by TXE. Once this | ||
// is supported, check the message here and delete try deleting the corresponding e2e test. | ||
// #[test(should_fail_with = "tx fee is higher than funded amount")] | ||
#[test(should_fail)] | ||
unconstrained fn setup_refund_insufficient_funded_amount() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,18 +39,39 @@ global MAX_NOTE_ENCRYPTED_LOGS_PER_CALL: u32 = 16; | |
global MAX_ENCRYPTED_LOGS_PER_CALL: u32 = 4; // If modifying, update DEPLOYER_CONTRACT_ADDRESS. | ||
global MAX_UNENCRYPTED_LOGS_PER_CALL: u32 = 4; // If modifying, update DEPLOYER_CONTRACT_ADDRESS. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to re-order the constants here so that they get successfully evaluated in TS with the eval method --> the constants became interdependent |
||
// TREES RELATED CONSTANTS | ||
global ARCHIVE_HEIGHT: u32 = 16; | ||
global VK_TREE_HEIGHT: u32 = 5; | ||
global FUNCTION_TREE_HEIGHT: u32 = 5; | ||
global NOTE_HASH_TREE_HEIGHT: u32 = 32; | ||
global PUBLIC_DATA_TREE_HEIGHT: u32 = 40; | ||
global NULLIFIER_TREE_HEIGHT: u32 = 20; | ||
global L1_TO_L2_MSG_TREE_HEIGHT: u32 = 16; | ||
global ARTIFACT_FUNCTION_TREE_MAX_HEIGHT = 5; | ||
global NULLIFIER_TREE_ID = 0; | ||
global NOTE_HASH_TREE_ID = 1; | ||
global PUBLIC_DATA_TREE_ID = 2; | ||
global L1_TO_L2_MESSAGE_TREE_ID = 3; | ||
global ARCHIVE_TREE_ID = 4; | ||
|
||
// SUB-TREES RELATED CONSTANTS | ||
global NOTE_HASH_SUBTREE_HEIGHT: u32 = 6; | ||
global NULLIFIER_SUBTREE_HEIGHT: u32 = 6; | ||
global PUBLIC_DATA_SUBTREE_HEIGHT: u32 = 6; | ||
global L1_TO_L2_MSG_SUBTREE_HEIGHT: u32 = 4; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following few constants are newly computed. |
||
global NOTE_HASH_SUBTREE_SIBLING_PATH_LENGTH: u32 = NOTE_HASH_TREE_HEIGHT - NOTE_HASH_SUBTREE_HEIGHT; | ||
global NULLIFIER_SUBTREE_SIBLING_PATH_LENGTH: u32 = NULLIFIER_TREE_HEIGHT - NULLIFIER_SUBTREE_HEIGHT; | ||
global PUBLIC_DATA_SUBTREE_SIBLING_PATH_LENGTH: u32 = PUBLIC_DATA_TREE_HEIGHT - PUBLIC_DATA_SUBTREE_HEIGHT; | ||
global L1_TO_L2_MSG_SUBTREE_SIBLING_PATH_LENGTH: u32 = L1_TO_L2_MSG_TREE_HEIGHT - L1_TO_L2_MSG_SUBTREE_HEIGHT; | ||
|
||
// "PER TRANSACTION" CONSTANTS | ||
global MAX_NOTE_HASHES_PER_TX: u32 = 64; | ||
global MAX_NULLIFIERS_PER_TX: u32 = 64; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following couple of constants are newly computed. |
||
global MAX_NOTE_HASHES_PER_TX: u32 = (1 as u8 << NOTE_HASH_SUBTREE_HEIGHT as u8) as u32; | ||
global MAX_NULLIFIERS_PER_TX: u32 = (1 as u8 << NULLIFIER_SUBTREE_HEIGHT as u8) as u32; | ||
global MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX: u32 = 8; | ||
global MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX: u32 = 32; | ||
// If you touch any of the constants below don't forget to update MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX. | ||
global MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = 63; | ||
global PROTOCOL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = 1; | ||
// We cannot do constant propagation below and instead we have to hardcode the value because the generated code in TS | ||
// would then result in the type of the constant be a number and not a literal type. This would mess up the types. | ||
// Pain. | ||
global MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = 64; // MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX + PROTOCOL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX; | ||
global MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = (1 as u8 << PUBLIC_DATA_SUBTREE_HEIGHT as u8) as u32; | ||
global MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX - PROTOCOL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX; | ||
global MAX_PUBLIC_DATA_READS_PER_TX: u32 = 64; | ||
global MAX_L2_TO_L1_MSGS_PER_TX: u32 = 8; | ||
global MAX_NOTE_HASH_READ_REQUESTS_PER_TX: u32 = 64; | ||
|
@@ -73,31 +94,6 @@ global MAX_PUBLIC_DATA_HINTS: u32 = 128; | |
// ROLLUP CONTRACT CONSTANTS - constants used only in l1-contracts | ||
global NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP: u32 = 16; | ||
|
||
// TREES RELATED CONSTANTS | ||
global VK_TREE_HEIGHT: u32 = 5; | ||
global FUNCTION_TREE_HEIGHT: u32 = 5; | ||
global NOTE_HASH_TREE_HEIGHT: u32 = 32; | ||
global PUBLIC_DATA_TREE_HEIGHT: u32 = 40; | ||
global NULLIFIER_TREE_HEIGHT: u32 = 20; | ||
global L1_TO_L2_MSG_TREE_HEIGHT: u32 = 16; | ||
global ARTIFACT_FUNCTION_TREE_MAX_HEIGHT = 5; | ||
global NULLIFIER_TREE_ID = 0; | ||
global NOTE_HASH_TREE_ID = 1; | ||
global PUBLIC_DATA_TREE_ID = 2; | ||
global L1_TO_L2_MESSAGE_TREE_ID = 3; | ||
global ARCHIVE_TREE_ID = 4; | ||
|
||
// SUB-TREES RELATED CONSTANTS | ||
global NOTE_HASH_SUBTREE_HEIGHT: u32 = 6; | ||
global NOTE_HASH_SUBTREE_SIBLING_PATH_LENGTH: u32 = 26; | ||
global NULLIFIER_SUBTREE_HEIGHT: u32 = 6; | ||
global PUBLIC_DATA_SUBTREE_HEIGHT: u32 = 6; | ||
global ARCHIVE_HEIGHT: u32 = 16; | ||
global NULLIFIER_SUBTREE_SIBLING_PATH_LENGTH: u32 = 14; | ||
global PUBLIC_DATA_SUBTREE_SIBLING_PATH_LENGTH: u32 = 34; | ||
global L1_TO_L2_MSG_SUBTREE_HEIGHT: u32 = 4; | ||
global L1_TO_L2_MSG_SUBTREE_SIBLING_PATH_LENGTH: u32 = 12; | ||
|
||
// VK TREE CONSTANTS | ||
global PRIVATE_KERNEL_INIT_INDEX: u32 = 0; | ||
global PRIVATE_KERNEL_INNER_INDEX: u32 = 1; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -229,7 +229,7 @@ function generateCppConstants({ constants }: ParsedContent, targetPath: string) | |
#pragma once | ||
|
||
${processConstantsCpp(constants)} | ||
\n`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a minor bug -- there were 2 new lines inserted at the end of the file aztec-constants.hpp file. |
||
`; | ||
|
||
fs.writeFileSync(targetPath, resultCpp); | ||
} | ||
|
@@ -322,6 +322,9 @@ function evaluateExpressions(expressions: [string, string][]): { [key: string]: | |
const prelude = expressions | ||
.map(([name, rhs]) => { | ||
const guardedRhs = rhs | ||
// Remove 'as u8' and 'as u32' castings | ||
.replaceAll(' as u8', '') | ||
.replaceAll(' as u32', '') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the bit shifts I had to introduce castings and this gets rid of them in TS. |
||
// We make some space around the parentheses, so that constant numbers are still split. | ||
.replace(/\(/g, '( ') | ||
.replace(/\)/g, ' )') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -146,4 +146,4 @@ | |
"testRegex": "./src/.*\\.test\\.(js|mjs|ts)$", | ||
"rootDir": "./src" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just run yarn formatting in yarn-projects and this happened. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,10 +51,10 @@ describe('e2e_auth_contract', () => { | |
expect(await contract.methods.get_authorized().simulate()).toEqual(AztecAddress.ZERO); | ||
}); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another random change. It was unnecessary to use send and wait here. |
||
it('non-admin canoot set authorized', async () => { | ||
await expect( | ||
contract.withWallet(other).methods.set_authorized(authorized.getAddress()).send().wait(), | ||
).rejects.toThrow('caller is not admin'); | ||
it('non-admin cannot set authorized', async () => { | ||
await expect(contract.withWallet(other).methods.set_authorized(authorized.getAddress()).prove()).rejects.toThrow( | ||
'caller is not admin', | ||
); | ||
}); | ||
|
||
it('admin sets authorized', async () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -350,7 +350,7 @@ export class FeesTest { | |
await this.snapshotManager.snapshot( | ||
'fund_alice', | ||
async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The bigint conversions here were unnecessary. |
||
await this.mintPrivateBananas(BigInt(this.ALICE_INITIAL_BANANAS), this.aliceAddress); | ||
await this.mintPrivateBananas(this.ALICE_INITIAL_BANANAS, this.aliceAddress); | ||
await this.bananaCoin.methods.mint_public(this.aliceAddress, this.ALICE_INITIAL_BANANAS).send().wait(); | ||
}, | ||
() => Promise.resolve(), | ||
|
@@ -361,7 +361,7 @@ export class FeesTest { | |
await this.snapshotManager.snapshot( | ||
'fund_alice_with_tokens', | ||
async () => { | ||
await this.mintTokenWithRefunds(BigInt(this.ALICE_INITIAL_BANANAS)); | ||
await this.mintTokenWithRefunds(this.ALICE_INITIAL_BANANAS); | ||
}, | ||
() => Promise.resolve(), | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to re-order the constants here so that they get successfully evaluated in TS with the eval method --> the constants became interdependent