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.

Sign up for GitHub

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

chore: fix formatting that fails CI run #5942

Merged
merged 3 commits into from
Apr 23, 2024
Merged

chore: fix formatting that fails CI run #5942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3a0eae
Fix formatting
nventuro Apr 23, 2024
78fbdfd
fix: serialization issue with roles
LHerskind Apr 23, 2024
66e379e
fix: deserialization issue with roles if imported
LHerskind Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/roles.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use dep::aztec::protocol_types::traits::{FromField, ToField, Serialize};
use dep::aztec::protocol_types::traits::{FromField, ToField, Serialize, Deserialize};

global ADMIN_FLAG: u64 = 1;
global MINTER_FLAG: u64 = 2;
Expand Down Expand Up @@ -41,14 +41,26 @@ impl ToField for UserFlags {
}
}

// We don't actually need to implement this trait, but the current macros system requires that all types that are
// contained by a state variable are serializable in order to determine their length.
// Once https://github.com/AztecProtocol/aztec-packages/issues/5736 is closed we'll be able to remove this (unless
// https://github.com/AztecProtocol/aztec-packages/issues/5491 is addressed first, in which case we'd remove the to/from
// field traits instead).
impl Serialize<1> for UserFlags {
fn serialize(self) -> [Field; 1] {
[self.to_field()]
// We implement this as it is used when serializing the state variable into return values
// This is very inefficient if used to store the state variable.
// We are currently "abusing" that the `to_field` is called in the `scheduled_value_change`
// where we are using this value.
impl Serialize<3> for UserFlags {
fn serialize(self) -> [Field; 3] {
[self.is_admin.to_field(), self.is_minter.to_field(), self.is_blacklisted.to_field()]
// [self.to_field()]
}
}

// We implement this as it is required for other contracts to be able to "rebuild" the variable
// after having received the serialized value as a return-value
impl Deserialize<3> for UserFlags {
fn deserialize(fields: [Field; 3]) -> Self {
Self {
is_admin: fields[0] as bool,
is_minter: fields[1] as bool,
is_blacklisted: fields[2] as bool,
}
}
}

Expand Down
73 changes: 27 additions & 46 deletions yarn-project/end-to-end/src/e2e_blacklist_token_contract.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,28 +68,11 @@ describe('e2e_blacklist_token_contract', () => {
return this;
}

toCallValue() {
toNoirStruct() {
// We need to use lowercase identifiers as those are what the noir interface expects
// eslint-disable-next-line camelcase
return { is_admin: this.isAdmin, is_minter: this.isMinter, is_blacklisted: this.isBlacklisted };
}

toReturnValue() {
// This matches the result of the serialize() function in the Noir struct

let value = 0;
if (this.isAdmin) {
value += 1;
}

if (this.isMinter) {
value += 2;
}

if (this.isBlacklisted) {
value += 4;
}

return [BigInt(value)];
}
}

const addPendingShieldNoteToPXE = async (accountIndex: number, amount: bigint, secretHash: Fr, txHash: TxHash) => {
Expand Down Expand Up @@ -119,9 +102,7 @@ describe('e2e_blacklist_token_contract', () => {

await mineBlocks(DELAY); // This gets us past the block of change

expect(await asset.methods.get_roles(admin.getAddress()).simulate()).toEqual(
new Role().withAdmin().toReturnValue(),
);
expect(await asset.methods.get_roles(admin.getAddress()).simulate()).toEqual(new Role().withAdmin().toNoirStruct());

logger.info(`Token deployed to ${asset.address}`);
tokenSim = new TokenSimulator(
Expand Down Expand Up @@ -151,59 +132,59 @@ describe('e2e_blacklist_token_contract', () => {
const adminMinterRole = new Role().withAdmin().withMinter();
await asset
.withWallet(admin)
.methods.update_roles(admin.getAddress(), adminMinterRole.toCallValue())
.methods.update_roles(admin.getAddress(), adminMinterRole.toNoirStruct())
.send()
.wait();

await mineBlocks(DELAY); // This gets us past the block of change

expect(await asset.methods.get_roles(admin.getAddress()).simulate()).toEqual(adminMinterRole.toReturnValue());
expect(await asset.methods.get_roles(admin.getAddress()).simulate()).toEqual(adminMinterRole.toNoirStruct());
});

it('create a new admin', async () => {
const adminRole = new Role().withAdmin();
await asset.withWallet(admin).methods.update_roles(other.getAddress(), adminRole.toCallValue()).send().wait();
await asset.withWallet(admin).methods.update_roles(other.getAddress(), adminRole.toNoirStruct()).send().wait();

await mineBlocks(DELAY); // This gets us past the block of change

expect(await asset.methods.get_roles(other.getAddress()).simulate()).toEqual(adminRole.toReturnValue());
expect(await asset.methods.get_roles(other.getAddress()).simulate()).toEqual(adminRole.toNoirStruct());
});

it('revoke the new admin', async () => {
const noRole = new Role();
await asset.withWallet(admin).methods.update_roles(other.getAddress(), noRole.toCallValue()).send().wait();
await asset.withWallet(admin).methods.update_roles(other.getAddress(), noRole.toNoirStruct()).send().wait();

await mineBlocks(DELAY); // This gets us past the block of change

expect(await asset.methods.get_roles(other.getAddress()).simulate()).toEqual(noRole.toReturnValue());
expect(await asset.methods.get_roles(other.getAddress()).simulate()).toEqual(noRole.toNoirStruct());
});

it('blacklist account', async () => {
const blacklistRole = new Role().withBlacklisted();
await asset
.withWallet(admin)
.methods.update_roles(blacklisted.getAddress(), blacklistRole.toCallValue())
.methods.update_roles(blacklisted.getAddress(), blacklistRole.toNoirStruct())
.send()
.wait();

await mineBlocks(DELAY); // This gets us past the block of change

expect(await asset.methods.get_roles(blacklisted.getAddress()).simulate()).toEqual(blacklistRole.toReturnValue());
expect(await asset.methods.get_roles(blacklisted.getAddress()).simulate()).toEqual(blacklistRole.toNoirStruct());
});

describe('failure cases', () => {
it('set roles from non admin', async () => {
const newRole = new Role().withAdmin().withAdmin();
await expect(
asset.withWallet(other).methods.update_roles(AztecAddress.random(), newRole.toCallValue()).prove(),
).rejects.toThrow("Assertion failed: caller is not admin 'caller_roles.isAdmin'");
asset.withWallet(other).methods.update_roles(AztecAddress.random(), newRole.toNoirStruct()).prove(),
).rejects.toThrow("Assertion failed: caller is not admin 'caller_roles.is_admin'");
});

it('revoke minter from non admin', async () => {
const noRole = new Role();
await expect(
asset.withWallet(other).methods.update_roles(admin.getAddress(), noRole.toCallValue()).prove(),
).rejects.toThrow("Assertion failed: caller is not admin 'caller_roles.isAdmin'");
asset.withWallet(other).methods.update_roles(admin.getAddress(), noRole.toNoirStruct()).prove(),
).rejects.toThrow("Assertion failed: caller is not admin 'caller_roles.is_admin'");
});
});
});
Expand Down Expand Up @@ -253,7 +234,7 @@ describe('e2e_blacklist_token_contract', () => {
it('mint to blacklisted entity', async () => {
await expect(
asset.withWallet(wallets[1]).methods.mint_public(blacklisted.getAddress(), 1n).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.is_blacklisted'");
});
});
});
Expand Down Expand Up @@ -325,7 +306,7 @@ describe('e2e_blacklist_token_contract', () => {

it('mint and try to redeem at blacklist', async () => {
await expect(asset.methods.redeem_shield(blacklisted.getAddress(), amount, secret).prove()).rejects.toThrow(
"Assertion failed: Blacklisted: Recipient '!to_roles.isBlacklisted'",
"Assertion failed: Blacklisted: Recipient '!to_roles.is_blacklisted'",
);
});
});
Expand Down Expand Up @@ -482,13 +463,13 @@ describe('e2e_blacklist_token_contract', () => {
it('transfer from a blacklisted account', async () => {
await expect(
asset.methods.transfer_public(blacklisted.getAddress(), wallets[0].getAddress(), 1n, 0n).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.is_blacklisted'");
});

it('transfer to a blacklisted account', async () => {
await expect(
asset.methods.transfer_public(wallets[0].getAddress(), blacklisted.getAddress(), 1n, 0n).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.is_blacklisted'");
});
});
});
Expand Down Expand Up @@ -643,13 +624,13 @@ describe('e2e_blacklist_token_contract', () => {
it('transfer from a blacklisted account', async () => {
await expect(
asset.methods.transfer(blacklisted.getAddress(), wallets[0].getAddress(), 1n, 0).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.is_blacklisted'");
});

it('transfer to a blacklisted account', async () => {
await expect(
asset.methods.transfer(wallets[0].getAddress(), blacklisted.getAddress(), 1n, 0).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.is_blacklisted'");
});
});
});
Expand Down Expand Up @@ -770,7 +751,7 @@ describe('e2e_blacklist_token_contract', () => {
it('shielding from blacklisted account', async () => {
await expect(
asset.withWallet(blacklisted).methods.shield(blacklisted.getAddress(), 1n, secretHash, 0).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.is_blacklisted'");
});
});
});
Expand Down Expand Up @@ -884,13 +865,13 @@ describe('e2e_blacklist_token_contract', () => {
it('unshield from blacklisted account', async () => {
await expect(
asset.methods.unshield(blacklisted.getAddress(), wallets[0].getAddress(), 1n, 0).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Sender '!from_roles.is_blacklisted'");
});

it('unshield to blacklisted account', async () => {
await expect(
asset.methods.unshield(wallets[0].getAddress(), blacklisted.getAddress(), 1n, 0).prove(),
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.isBlacklisted'");
).rejects.toThrow("Assertion failed: Blacklisted: Recipient '!to_roles.is_blacklisted'");
});
});
});
Expand Down Expand Up @@ -987,7 +968,7 @@ describe('e2e_blacklist_token_contract', () => {

it('burn from blacklisted account', async () => {
await expect(asset.methods.burn_public(blacklisted.getAddress(), 1n, 0).prove()).rejects.toThrow(
"Assertion failed: Blacklisted: Sender '!from_roles.isBlacklisted'",
"Assertion failed: Blacklisted: Sender '!from_roles.is_blacklisted'",
);
});
});
Expand Down Expand Up @@ -1105,7 +1086,7 @@ describe('e2e_blacklist_token_contract', () => {

it('burn from blacklisted account', async () => {
await expect(asset.methods.burn(blacklisted.getAddress(), 1n, 0).prove()).rejects.toThrow(
"Assertion failed: Blacklisted: Sender '!from_roles.isBlacklisted'",
"Assertion failed: Blacklisted: Sender '!from_roles.is_blacklisted'",
);
});
});
Expand Down
Loading