From 0f361bbea6f37893ec9d766c154942d246d83500 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 23 May 2025 10:14:42 +0000 Subject: [PATCH 01/24] WIP --- .../aztec-nr/uint-note/src/uint_note.nr | 9 +- noir-projects/noir-contracts/Nargo.toml | 1 + .../app/orderbook_contract/Nargo.toml | 10 + .../app/orderbook_contract/src/config.nr | 10 + .../app/orderbook_contract/src/main.nr | 193 ++++++++++++++++++ .../app/orderbook_contract/src/order.nr | 13 ++ .../end-to-end/src/e2e_orderbook.test.ts | 115 +++++++++++ 7 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 noir-projects/noir-contracts/contracts/app/orderbook_contract/Nargo.toml create mode 100644 noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr create mode 100644 noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr create mode 100644 noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr create mode 100644 yarn-project/end-to-end/src/e2e_orderbook.test.ts diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index 0919b7b0caca..39222d82be0f 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -207,7 +207,7 @@ impl NoteType for PrivateUintPartialNotePrivateLogContent { /// slot, but the value field has not yet been set. A partial note can be completed in public with the `complete` /// function (revealing the value to the public), resulting in a UintNote that can be used like any other one (except /// of course that its value is known). -#[derive(Packable, Serialize, Deserialize)] +#[derive(Packable, Serialize, Deserialize, Eq)] pub struct PartialUintNote { commitment: Field, } @@ -268,6 +268,13 @@ impl PartialUintNote { } } +// We implement ToField for PartialUintNote to allow it to be used as a key in a Map (e.g. in the Orderbook contract). +impl ToField for PartialUintNote { + fn to_field(self) -> Field { + self.commitment + } +} + mod test { use super::{ PartialUintNote, PrivateUintPartialNotePrivateLogContent, UintNote, diff --git a/noir-projects/noir-contracts/Nargo.toml b/noir-projects/noir-contracts/Nargo.toml index 9e6c33825e5a..2a42a1c76410 100644 --- a/noir-projects/noir-contracts/Nargo.toml +++ b/noir-projects/noir-contracts/Nargo.toml @@ -16,6 +16,7 @@ members = [ "contracts/app/escrow_contract", "contracts/app/lending_contract", "contracts/app/nft_contract", + "contracts/app/orderbook_contract", "contracts/app/price_feed_contract", "contracts/app/token_blacklist_contract", "contracts/app/token_bridge_contract", diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/Nargo.toml b/noir-projects/noir-contracts/contracts/app/orderbook_contract/Nargo.toml new file mode 100644 index 000000000000..baa2fa172a00 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/Nargo.toml @@ -0,0 +1,10 @@ +[package] +name = "orderbook_contract" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../../aztec-nr/aztec" } +token = { path = "../token_contract" } +uint_note = { path = "../../../../aztec-nr/uint-note" } diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr new file mode 100644 index 000000000000..b66333b6b3ae --- /dev/null +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -0,0 +1,10 @@ +use dep::aztec::protocol_types::{address::AztecAddress, traits::{Deserialize, Packable, Serialize}}; +use std::meta::derive; + +/// We store the tokens of the DEX in a struct such that to load it from PublicImmutable asserts only a single +/// merkle proof. +#[derive(Deserialize, Eq, Packable, Serialize)] +pub struct Config { + pub token0: AztecAddress, + pub token1: AztecAddress, +} diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr new file mode 100644 index 000000000000..6350aa00ea8c --- /dev/null +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -0,0 +1,193 @@ +mod config; +mod order; + +use aztec::macros::aztec; + +/// ## Overview +/// This contract demonstrates how to implement an **Orderbook** that maintains **public state** +/// while still achieving **identity privacy**. However, it does **not provide function privacy**: +/// - Anyone can observe **what actions** were performed. +/// - All amounts involved are visible, but **who** performed the action remains private. +/// +/// **Note:** +/// This is purely a demonstration implemented to test various features of Aztec.nr. The **Aztec team** does not +/// consider this the optimal design for building a DEX. +/// +/// ## Reentrancy Guard Considerations +/// +/// ### 1. Private Functions: +/// Reentrancy protection is typically necessary if entering an intermediate state that is only valid when +/// the action completes uninterrupted. This follows the **Checks-Effects-Interactions** pattern. +/// +/// - In this contract, **private functions** do not introduce intermediate states. +/// - All operations will be fully executed in **public** without needing intermediate checks. +/// +/// ### 2. Public Functions: +/// No **reentrancy guard** is required for public functions because: +/// - All public functions are marked as **internal** with a **single callsite** - from a private function. +/// - Public functions **cannot call private functions**, eliminating the risk of reentering into them from private. +/// - Since public functions are internal-only, **external contracts cannot access them**, ensuring no external +/// contract can trigger a reentrant call. This eliminates the following attack vector: +/// `Orderbook.private_fn --> Orderbook.public_fn --> ExternalContract.fn --> Orderbook.public_fn`. +#[aztec] +pub contract Orderbook { + use crate::{config::Config, order::Order}; + use aztec::{ + event::event_interface::EventInterface, + macros::{ + events::event, + functions::{initializer, internal, private, public, utility}, + storage::storage, + }, + prelude::{AztecAddress, Map, PublicImmutable}, + protocol_types::traits::{Serialize, ToField}, + unencrypted_logs::unencrypted_event_emission::encode_event, + }; + + use token::Token; + use uint_note::uint_note::PartialUintNote; + + // The event contains only the `order_id` as the order itself can be retrieved via the `get_order` function. + #[derive(Serialize)] + #[event] + struct OrderCreated { + order_id: PartialUintNote, + } + + #[derive(Serialize)] + #[event] + struct OrderFulfilled { + order_id: PartialUintNote, + } + + #[storage] + struct Storage { + config: PublicImmutable, + orders: Map, Context>, + } + + #[public] + #[initializer] + fn constructor(token0: AztecAddress, token1: AztecAddress) { + storage.config.initialize(Config { token0, token1 }); + } + + /// Privately creates a new order in the orderbook + /// The maker specifies the tokens and amounts they want to trade + #[private] + fn create_order( + token_in: AztecAddress, + token_out: AztecAddress, + amount_in: u128, + amount_out: u128, + nonce: Field, + ) { + let config = storage.config.read(); + + assert((token_in == config.token0) | (token_in == config.token1), "TOKEN_IN_IS_INVALID"); + assert((token_out == config.token0) | (token_out == config.token1), "TOKEN_OUT_IS_INVALID"); + assert(token_in != token_out, "SAME_TOKEN_TRADE"); + assert(amount_in > 0 as u128, "ZERO_AMOUNT_IN"); + assert(amount_out > 0 as u128, "ZERO_AMOUNT_OUT"); + + let maker = context.msg_sender(); + + // Transfer tokens from maker to the public balance of this contract + Token::at(token_in) + .transfer_to_public(maker, context.this_address(), amount_in, nonce) + .call(&mut context); + + // Prepare a partial note that will get completed once the order is fulfilled. + let maker_partial_note = + Token::at(token_out).prepare_private_balance_increase(maker, maker).call(&mut context); + + // Create and store the order + let order = Order { amount_in, amount_out, zero_to_one: token_in == config.token0 }; + + Orderbook::at(context.this_address())._create_order(maker_partial_note, order).enqueue( + &mut context, + ); + } + + #[public] + #[internal] + fn _create_order(order_id: PartialUintNote, order: Order) { + // The partial note serves as a natural unique identifier for the order. While a maker could theoretically + // create duplicate order IDs by manipulating the randomness, this would be self-defeating - only one order + // with a given ID can be fulfilled, meaning any duplicates would result in the maker's tokens being + // irretrievably locked in the public balance of this contract. + storage.orders.at(order_id).initialize(order); + + OrderCreated { order_id }.emit(encode_event(&mut context)); + } + + /// Privately fulfills an existing order in the orderbook + /// The taker provides the order ID they want to fulfill + #[private] + fn fulfill_order(order_id: PartialUintNote, nonce: Field) { + let config = storage.config.read(); + let order = storage.orders.at(order_id).read(); + let taker = context.msg_sender(); + + // Determine which tokens are being exchanged based on zero_to_one flag + let (token_in, token_out) = if order.zero_to_one { + (config.token0, config.token1) + } else { + (config.token1, config.token0) + }; + + // Transfer the amount_out from taker to the contract + Token::at(token_out) + .transfer_to_public(taker, context.this_address(), order.amount_out, nonce) + .call(&mut context); + + // Prepare partial note for taker to receive token_in + let taker_partial_note = + Token::at(token_in).prepare_private_balance_increase(taker, taker).call(&mut context); + + // Nullify the order such that it cannot be fulfilled again. We emit a nullifier instead of deleting the order + // from public storage because we get no refund for resetting public storage to zero and just emitting + // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order). We use the `order_id` + // itself as the nullifier because this contract does not work with notes and hence there is no risk of + // colliding with a real note nullifier. + context.push_nullifier(order_id.to_field()); + + // Enqueue the fulfillment to finalize both partial notes + Orderbook::at(context.this_address()) + ._fulfill_order( + order_id, + taker_partial_note, + token_in, + token_out, + order.amount_in, + order.amount_out, + ) + .enqueue(&mut context); + } + + #[public] + #[internal] + fn _fulfill_order( + order_id: PartialUintNote, // maker partial note is used as `order_id` + taker_partial_note: PartialUintNote, + token_in: AztecAddress, + token_out: AztecAddress, + amount_in: u128, + amount_out: u128, + ) { + // Finalize transfer of amount_out of token_out to maker + Token::at(token_out).finalize_transfer_to_private(amount_out, order_id).call(&mut context); + + // Finalize transfer of of amount_in of token_in to taker + Token::at(token_in).finalize_transfer_to_private(amount_in, taker_partial_note).call( + &mut context, + ); + + OrderFulfilled { order_id }.emit(encode_event(&mut context)); + } + + #[utility] + unconstrained fn get_order(order_id: PartialUintNote) -> pub Order { + storage.orders.at(order_id).read() + } +} diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr new file mode 100644 index 000000000000..c08f1f3be9b6 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr @@ -0,0 +1,13 @@ +use aztec::protocol_types::traits::{Deserialize, Packable, Serialize}; + +// TODO: We do not necessarily need full 128 bits for the amounts so we could try to pack the whole order into 1 Field +// and save on public storage costs. +#[derive(Deserialize, Eq, Packable, Serialize)] +pub struct Order { + // Amount of input tokens + pub amount_in: u128, + // Amount of output tokens + pub amount_out: u128, + // Whether the order is from token0 to token1 or from token1 to token0 + pub zero_to_one: bool, +} diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts new file mode 100644 index 000000000000..cc0a5be31098 --- /dev/null +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -0,0 +1,115 @@ +import { type AccountWallet, type FieldLike, Fr, type Logger, type PXE } from '@aztec/aztec.js'; +import { type OrderCreated, type OrderFulfilled, OrderbookContract } from '@aztec/noir-contracts.js/Orderbook'; +import type { TokenContract } from '@aztec/noir-contracts.js/Token'; + +import { jest } from '@jest/globals'; + +import { deployToken, mintTokensToPrivate } from './fixtures/token_utils.js'; +import { setup } from './fixtures/utils.js'; + +const TIMEOUT = 120_000; + +describe('Orderbook', () => { + jest.setTimeout(TIMEOUT); + + let teardown: () => Promise; + let logger: Logger; + + let pxe: PXE; + + let adminWallet: AccountWallet; + let maker: AccountWallet; + let taker: AccountWallet; + + let token0: TokenContract; + let token1: TokenContract; + let orderbook: OrderbookContract; + + const amountIn = 1000n; + const amountOut = 2000n; + + beforeAll(async () => { + ({ + pxe, + teardown, + wallets: [adminWallet, maker, taker], + logger, + } = await setup(3)); + + token0 = await deployToken(adminWallet, 0n, logger); + token1 = await deployToken(adminWallet, 0n, logger); + + orderbook = await OrderbookContract.deploy(adminWallet, token0.address, token1.address).send().deployed(); + + // Mint tokens to maker and taker + await mintTokensToPrivate(token0, adminWallet, maker.getAddress(), amountIn); + await mintTokensToPrivate(token1, adminWallet, taker.getAddress(), amountOut); + }); + + afterAll(() => teardown()); + + describe('full flow - happy path', () => { + let orderId: { + commitment: FieldLike; + }; + + it('creates an order', async () => { + const nonceForAuthwits = Fr.random(); + + // Create authwit for maker to allow orderbook to transfer amountIn of token0 to itself + const makerAuthwit = await maker.createAuthWit({ + caller: orderbook.address, + action: token0.methods.transfer_to_public(maker.getAddress(), orderbook.address, amountIn, nonceForAuthwits), + }); + + // Create order + await orderbook + .withWallet(maker) + .methods.create_order(token0.address, token1.address, amountIn, amountOut, nonceForAuthwits) + .with({ authWitnesses: [makerAuthwit] }) + .send() + .wait(); + + const orderCreatedEvents = await pxe.getPublicEvents(OrderbookContract.events.OrderCreated, 0, 100); + expect(orderCreatedEvents.length).toBe(1); + + // Get order ID from emitted event + orderId = orderCreatedEvents[0].order_id; + + // Get order from orderbook and verify details + const order = await orderbook.methods.get_order(orderId).simulate(); + expect(order.amount_in).toEqual(amountIn); + expect(order.amount_out).toEqual(amountOut); + expect(order.zero_to_one).toBeTruthy(); + }); + + it('fulfills an order', async () => { + const nonceForAuthwits = Fr.random(); + + // Create authwit for taker to allow orderbook to transfer amountOut of token1 to itself + const takerAuthwit = await taker.createAuthWit({ + caller: orderbook.address, + action: token1.methods.transfer_to_public(taker.getAddress(), orderbook.address, amountOut, nonceForAuthwits), + }); + + // Fulfill order + await orderbook + .withWallet(taker) + .methods.fulfill_order(orderId, nonceForAuthwits) + .with({ authWitnesses: [takerAuthwit] }) + .send() + .wait(); + + // Verify order was fulfilled by checking events + const orderFulfilledEvents = await pxe.getPublicEvents( + OrderbookContract.events.OrderFulfilled, + 0, + 100, + ); + expect(orderFulfilledEvents.length).toBe(1); + expect(orderFulfilledEvents[0].order_id).toEqual(orderId); + + // Verify balances after order fulfillment + }); + }); +}); From b3517f46854789d66b77f4778f139fa05933d5c3 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 23 May 2025 10:28:50 +0000 Subject: [PATCH 02/24] wip --- .../end-to-end/src/e2e_orderbook.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index cc0a5be31098..fdcba178b21c 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -81,6 +81,13 @@ describe('Orderbook', () => { expect(order.amount_in).toEqual(amountIn); expect(order.amount_out).toEqual(amountOut); expect(order.zero_to_one).toBeTruthy(); + + // At this point, amountIn of token0 should be transferred to the public balance of the orderbook and maker + // should have 0. + const orderbookBalances0 = await token0.withWallet(maker).methods.balance_of_public(orderbook.address).simulate(); + const makerBalances0 = await token0.withWallet(maker).methods.balance_of_private(maker.getAddress()).simulate(); + expect(orderbookBalances0).toEqual(amountIn); + expect(makerBalances0).toEqual(0n); }); it('fulfills an order', async () => { @@ -110,6 +117,19 @@ describe('Orderbook', () => { expect(orderFulfilledEvents[0].order_id).toEqual(orderId); // Verify balances after order fulfillment + const makerBalances0 = await token0.withWallet(maker).methods.balance_of_private(maker.getAddress()).simulate(); + const makerBalances1 = await token1.withWallet(maker).methods.balance_of_private(maker.getAddress()).simulate(); + const takerBalances0 = await token0.withWallet(taker).methods.balance_of_private(taker.getAddress()).simulate(); + const takerBalances1 = await token1.withWallet(taker).methods.balance_of_private(taker.getAddress()).simulate(); + + // Full maker token 0 balance should be transferred to taker and hence maker should have 0 + expect(makerBalances0).toEqual(0n); + // amountOut of token1 should be transferred to maker + expect(makerBalances1).toEqual(amountOut); + // amountIn of token0 should be transferred to taker + expect(takerBalances0).toEqual(amountIn); + // Full taker token 1 balance should be transferred to maker and hence taker should have 0 + expect(takerBalances1).toEqual(0n); }); }); }); From 6d71ec194419fce6edf5abb5a1fb08b2cdfd6ff9 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 23 May 2025 10:36:37 +0000 Subject: [PATCH 03/24] justification of my laziness --- yarn-project/end-to-end/src/e2e_orderbook.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index fdcba178b21c..e6f4c24a14ed 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -9,6 +9,9 @@ import { setup } from './fixtures/utils.js'; const TIMEOUT = 120_000; +// We test only a happy path here since this is just a demonstration of the orderbook contract and writing here more +// thorough tests seems unnecessary. Also hopefully we will migrate this to TXE eventually. Didn't write it in TXE +// now as there is no way to obtain public events and all of TXE will be rewritten soon. describe('Orderbook', () => { jest.setTimeout(TIMEOUT); From 8f3edb87e7766e18eff4a89b7407c859abb04c0b Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 23 May 2025 10:48:36 +0000 Subject: [PATCH 04/24] better comments --- .../contracts/app/orderbook_contract/src/main.nr | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 6350aa00ea8c..02d2c7cf1284 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -101,7 +101,7 @@ pub contract Orderbook { let maker_partial_note = Token::at(token_out).prepare_private_balance_increase(maker, maker).call(&mut context); - // Create and store the order + // Create the order and store it in public storage let order = Order { amount_in, amount_out, zero_to_one: token_in == config.token0 }; Orderbook::at(context.this_address())._create_order(maker_partial_note, order).enqueue( @@ -150,6 +150,11 @@ pub contract Orderbook { // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order). We use the `order_id` // itself as the nullifier because this contract does not work with notes and hence there is no risk of // colliding with a real note nullifier. + // + // It's worth noting that since nullifier information is public, a malicious actor could potentially frontrun + // the taker's transaction. While this vulnerability exists in this simplified demonstration, a production + // implementation would typically employ a privileged matchmaker service to securely pair makers and takers, + // thereby ensuring order execution integrity. context.push_nullifier(order_id.to_field()); // Enqueue the fulfillment to finalize both partial notes From a9f4573dae8315b47e354db6372aaecc5d2e453e Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 23 May 2025 11:07:25 +0000 Subject: [PATCH 05/24] linking issue --- .../noir-contracts/contracts/app/orderbook_contract/src/main.nr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 02d2c7cf1284..d3cbe2691dca 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -136,6 +136,8 @@ pub contract Orderbook { (config.token1, config.token0) }; + // TODO(#14362): Once we allow partial note completion in private we can skip the transfer to public here and + // just finalize the maker's partial note straight away here. // Transfer the amount_out from taker to the contract Token::at(token_out) .transfer_to_public(taker, context.this_address(), order.amount_out, nonce) From 00604483c63baa9232ac4bbd858b1359be1f5584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Fri, 23 May 2025 15:17:26 +0200 Subject: [PATCH 06/24] Update noir-projects/aztec-nr/uint-note/src/uint_note.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- noir-projects/aztec-nr/uint-note/src/uint_note.nr | 1 - 1 file changed, 1 deletion(-) diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index 39222d82be0f..465987d99233 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -268,7 +268,6 @@ impl PartialUintNote { } } -// We implement ToField for PartialUintNote to allow it to be used as a key in a Map (e.g. in the Orderbook contract). impl ToField for PartialUintNote { fn to_field(self) -> Field { self.commitment From 1edeb218b517a1374cc55e31385c82392ceb35e1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 23 May 2025 13:34:22 +0000 Subject: [PATCH 07/24] not using PartialUintNote for order_id --- .../aztec-nr/uint-note/src/uint_note.nr | 8 +++- .../app/orderbook_contract/src/main.nr | 39 +++++++++++-------- .../end-to-end/src/e2e_orderbook.test.ts | 4 +- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index 465987d99233..8cfb9a73d10b 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -12,7 +12,7 @@ use dep::aztec::{ GENERATOR_INDEX__PARTIAL_NOTE_VALIDITY_COMMITMENT, }, hash::poseidon2_hash_with_separator, - traits::{Deserialize, Hash, Packable, Serialize, ToField}, + traits::{Deserialize, FromField, Hash, Packable, Serialize, ToField}, utils::arrays::array_concat, }, }; @@ -274,6 +274,12 @@ impl ToField for PartialUintNote { } } +impl FromField for PartialUintNote { + fn from_field(field: Field) -> Self { + Self { commitment: field } + } +} + mod test { use super::{ PartialUintNote, PrivateUintPartialNotePrivateLogContent, UintNote, diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index d3cbe2691dca..9013723cb986 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -40,7 +40,7 @@ pub contract Orderbook { storage::storage, }, prelude::{AztecAddress, Map, PublicImmutable}, - protocol_types::traits::{Serialize, ToField}, + protocol_types::traits::{FromField, Serialize, ToField}, unencrypted_logs::unencrypted_event_emission::encode_event, }; @@ -51,19 +51,19 @@ pub contract Orderbook { #[derive(Serialize)] #[event] struct OrderCreated { - order_id: PartialUintNote, + order_id: Field, } #[derive(Serialize)] #[event] struct OrderFulfilled { - order_id: PartialUintNote, + order_id: Field, } #[storage] struct Storage { config: PublicImmutable, - orders: Map, Context>, + orders: Map, Context>, } #[public] @@ -104,18 +104,18 @@ pub contract Orderbook { // Create the order and store it in public storage let order = Order { amount_in, amount_out, zero_to_one: token_in == config.token0 }; - Orderbook::at(context.this_address())._create_order(maker_partial_note, order).enqueue( - &mut context, - ); - } - - #[public] - #[internal] - fn _create_order(order_id: PartialUintNote, order: Order) { // The partial note serves as a natural unique identifier for the order. While a maker could theoretically // create duplicate order IDs by manipulating the randomness, this would be self-defeating - only one order // with a given ID can be fulfilled, meaning any duplicates would result in the maker's tokens being // irretrievably locked in the public balance of this contract. + let order_id = maker_partial_note.to_field(); + + Orderbook::at(context.this_address())._create_order(order_id, order).enqueue(&mut context); + } + + #[public] + #[internal] + fn _create_order(order_id: Field, order: Order) { storage.orders.at(order_id).initialize(order); OrderCreated { order_id }.emit(encode_event(&mut context)); @@ -124,7 +124,7 @@ pub contract Orderbook { /// Privately fulfills an existing order in the orderbook /// The taker provides the order ID they want to fulfill #[private] - fn fulfill_order(order_id: PartialUintNote, nonce: Field) { + fn fulfill_order(order_id: Field, nonce: Field) { let config = storage.config.read(); let order = storage.orders.at(order_id).read(); let taker = context.msg_sender(); @@ -157,7 +157,7 @@ pub contract Orderbook { // the taker's transaction. While this vulnerability exists in this simplified demonstration, a production // implementation would typically employ a privileged matchmaker service to securely pair makers and takers, // thereby ensuring order execution integrity. - context.push_nullifier(order_id.to_field()); + context.push_nullifier(order_id); // Enqueue the fulfillment to finalize both partial notes Orderbook::at(context.this_address()) @@ -175,15 +175,20 @@ pub contract Orderbook { #[public] #[internal] fn _fulfill_order( - order_id: PartialUintNote, // maker partial note is used as `order_id` + order_id: Field, taker_partial_note: PartialUintNote, token_in: AztecAddress, token_out: AztecAddress, amount_in: u128, amount_out: u128, ) { + // The `order_id` is a serialized form of the maker's partial note. + let maker_partial_note = PartialUintNote::from_field(order_id); + // Finalize transfer of amount_out of token_out to maker - Token::at(token_out).finalize_transfer_to_private(amount_out, order_id).call(&mut context); + Token::at(token_out).finalize_transfer_to_private(amount_out, maker_partial_note).call( + &mut context, + ); // Finalize transfer of of amount_in of token_in to taker Token::at(token_in).finalize_transfer_to_private(amount_in, taker_partial_note).call( @@ -194,7 +199,7 @@ pub contract Orderbook { } #[utility] - unconstrained fn get_order(order_id: PartialUintNote) -> pub Order { + unconstrained fn get_order(order_id: Field) -> pub Order { storage.orders.at(order_id).read() } } diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index e6f4c24a14ed..f06c341d7b66 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -52,9 +52,7 @@ describe('Orderbook', () => { afterAll(() => teardown()); describe('full flow - happy path', () => { - let orderId: { - commitment: FieldLike; - }; + let orderId: FieldLike; it('creates an order', async () => { const nonceForAuthwits = Fr.random(); From ae387df052442caf0c218739320900b26329702d Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 07:01:01 +0000 Subject: [PATCH 08/24] returning order_id --- .../contracts/app/orderbook_contract/src/main.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 9013723cb986..4d3079ea8f0b 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -81,7 +81,7 @@ pub contract Orderbook { amount_in: u128, amount_out: u128, nonce: Field, - ) { + ) -> Field { let config = storage.config.read(); assert((token_in == config.token0) | (token_in == config.token1), "TOKEN_IN_IS_INVALID"); @@ -111,6 +111,8 @@ pub contract Orderbook { let order_id = maker_partial_note.to_field(); Orderbook::at(context.this_address())._create_order(order_id, order).enqueue(&mut context); + + order_id } #[public] From 5eaf2e6a7afc83fdd550e57224650928b8ee0543 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 07:26:38 +0000 Subject: [PATCH 09/24] moving checks to Config --- .../app/orderbook_contract/src/config.nr | 31 +++++++++++++++++-- .../app/orderbook_contract/src/main.nr | 21 ++++--------- .../app/orderbook_contract/src/order.nr | 20 +++++++++++- .../end-to-end/src/e2e_orderbook.test.ts | 3 ++ 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index b66333b6b3ae..9b1e46acdfeb 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -5,6 +5,33 @@ use std::meta::derive; /// merkle proof. #[derive(Deserialize, Eq, Packable, Serialize)] pub struct Config { - pub token0: AztecAddress, - pub token1: AztecAddress, + token0: AztecAddress, + token1: AztecAddress, +} + +impl Config { + pub fn new(token0: AztecAddress, token1: AztecAddress) -> Self { + assert(!token0.eq(token1), "Tokens must be different"); + Self { token0, token1 } + } + + pub fn validate_input_tokens_and_get_direction( + self, + token_in: AztecAddress, + token_out: AztecAddress, + ) -> bool { + assert((token_in == self.token0) | (token_in == self.token1), "TOKEN_IN_IS_INVALID"); + assert((token_out == self.token0) | (token_out == self.token1), "TOKEN_OUT_IS_INVALID"); + assert(token_in != token_out, "SAME_TOKEN_TRADE"); + + token_in == self.token0 + } + + pub fn get_tokens(self, zero_to_one: bool) -> (AztecAddress, AztecAddress) { + if zero_to_one { + (self.token0, self.token1) + } else { + (self.token1, self.token0) + } + } } diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 4d3079ea8f0b..9e0e89efa581 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -69,7 +69,7 @@ pub contract Orderbook { #[public] #[initializer] fn constructor(token0: AztecAddress, token1: AztecAddress) { - storage.config.initialize(Config { token0, token1 }); + storage.config.initialize(Config::new(token0, token1)); } /// Privately creates a new order in the orderbook @@ -84,15 +84,12 @@ pub contract Orderbook { ) -> Field { let config = storage.config.read(); - assert((token_in == config.token0) | (token_in == config.token1), "TOKEN_IN_IS_INVALID"); - assert((token_out == config.token0) | (token_out == config.token1), "TOKEN_OUT_IS_INVALID"); - assert(token_in != token_out, "SAME_TOKEN_TRADE"); - assert(amount_in > 0 as u128, "ZERO_AMOUNT_IN"); - assert(amount_out > 0 as u128, "ZERO_AMOUNT_OUT"); + // Create the order (this validates the input tokens and amounts). + let order = Order::new(config, amount_in, amount_out, token_in, token_out); let maker = context.msg_sender(); - // Transfer tokens from maker to the public balance of this contract + // Transfer tokens from maker to the public balance of this contract. Token::at(token_in) .transfer_to_public(maker, context.this_address(), amount_in, nonce) .call(&mut context); @@ -101,15 +98,13 @@ pub contract Orderbook { let maker_partial_note = Token::at(token_out).prepare_private_balance_increase(maker, maker).call(&mut context); - // Create the order and store it in public storage - let order = Order { amount_in, amount_out, zero_to_one: token_in == config.token0 }; - // The partial note serves as a natural unique identifier for the order. While a maker could theoretically // create duplicate order IDs by manipulating the randomness, this would be self-defeating - only one order // with a given ID can be fulfilled, meaning any duplicates would result in the maker's tokens being // irretrievably locked in the public balance of this contract. let order_id = maker_partial_note.to_field(); + // Store the order in public storage and emit an event. Orderbook::at(context.this_address())._create_order(order_id, order).enqueue(&mut context); order_id @@ -132,11 +127,7 @@ pub contract Orderbook { let taker = context.msg_sender(); // Determine which tokens are being exchanged based on zero_to_one flag - let (token_in, token_out) = if order.zero_to_one { - (config.token0, config.token1) - } else { - (config.token1, config.token0) - }; + let (token_in, token_out) = config.get_tokens(order.zero_to_one); // TODO(#14362): Once we allow partial note completion in private we can skip the transfer to public here and // just finalize the maker's partial note straight away here. diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr index c08f1f3be9b6..5dde06f4152b 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr @@ -1,4 +1,5 @@ -use aztec::protocol_types::traits::{Deserialize, Packable, Serialize}; +use crate::config::Config; +use aztec::{prelude::AztecAddress, protocol_types::traits::{Deserialize, Packable, Serialize}}; // TODO: We do not necessarily need full 128 bits for the amounts so we could try to pack the whole order into 1 Field // and save on public storage costs. @@ -11,3 +12,20 @@ pub struct Order { // Whether the order is from token0 to token1 or from token1 to token0 pub zero_to_one: bool, } + +impl Order { + pub fn new( + config: Config, + amount_in: u128, + amount_out: u128, + token_in: AztecAddress, + token_out: AztecAddress, + ) -> Self { + assert(amount_in > 0 as u128, "ZERO_AMOUNT_IN"); + assert(amount_out > 0 as u128, "ZERO_AMOUNT_OUT"); + + let zero_to_one = config.validate_input_tokens_and_get_direction(token_in, token_out); + + Self { amount_in, amount_out, zero_to_one } + } +} diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index f06c341d7b66..a9a75d185824 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -74,6 +74,9 @@ describe('Orderbook', () => { const orderCreatedEvents = await pxe.getPublicEvents(OrderbookContract.events.OrderCreated, 0, 100); expect(orderCreatedEvents.length).toBe(1); + // TODO: Check that the order ID returned from create_order matches the one in the event. It's currently not + // supported by Aztec.js to get a return value from a sent transaction. + // Get order ID from emitted event orderId = orderCreatedEvents[0].order_id; From 9c3a5b0e383b63796d4950ba7e502bb9267aecd7 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 07:43:10 +0000 Subject: [PATCH 10/24] zero_to_one --> token_in_is_zero --- .../contracts/app/orderbook_contract/src/config.nr | 4 ++-- .../contracts/app/orderbook_contract/src/main.nr | 4 ++-- .../contracts/app/orderbook_contract/src/order.nr | 6 +++--- yarn-project/end-to-end/src/e2e_orderbook.test.ts | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index 9b1e46acdfeb..51f924816a73 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -27,8 +27,8 @@ impl Config { token_in == self.token0 } - pub fn get_tokens(self, zero_to_one: bool) -> (AztecAddress, AztecAddress) { - if zero_to_one { + pub fn get_tokens(self, token_in_is_zero: bool) -> (AztecAddress, AztecAddress) { + if token_in_is_zero { (self.token0, self.token1) } else { (self.token1, self.token0) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 9e0e89efa581..f2193c2cd7b7 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -126,8 +126,8 @@ pub contract Orderbook { let order = storage.orders.at(order_id).read(); let taker = context.msg_sender(); - // Determine which tokens are being exchanged based on zero_to_one flag - let (token_in, token_out) = config.get_tokens(order.zero_to_one); + // Determine which tokens are being exchanged based on token_in_is_zero flag + let (token_in, token_out) = config.get_tokens(order.token_in_is_zero); // TODO(#14362): Once we allow partial note completion in private we can skip the transfer to public here and // just finalize the maker's partial note straight away here. diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr index 5dde06f4152b..600944a8370e 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr @@ -10,7 +10,7 @@ pub struct Order { // Amount of output tokens pub amount_out: u128, // Whether the order is from token0 to token1 or from token1 to token0 - pub zero_to_one: bool, + pub token_in_is_zero: bool, } impl Order { @@ -24,8 +24,8 @@ impl Order { assert(amount_in > 0 as u128, "ZERO_AMOUNT_IN"); assert(amount_out > 0 as u128, "ZERO_AMOUNT_OUT"); - let zero_to_one = config.validate_input_tokens_and_get_direction(token_in, token_out); + let token_in_is_zero = config.validate_input_tokens_and_get_direction(token_in, token_out); - Self { amount_in, amount_out, zero_to_one } + Self { amount_in, amount_out, token_in_is_zero } } } diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index a9a75d185824..d5700370167a 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -84,7 +84,7 @@ describe('Orderbook', () => { const order = await orderbook.methods.get_order(orderId).simulate(); expect(order.amount_in).toEqual(amountIn); expect(order.amount_out).toEqual(amountOut); - expect(order.zero_to_one).toBeTruthy(); + expect(order.token_in_is_zero).toBeTruthy(); // At this point, amountIn of token0 should be transferred to the public balance of the orderbook and maker // should have 0. From 01d3dadb7d1972bcc48a3a809f09826a83c5f6cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Mon, 26 May 2025 09:56:54 +0200 Subject: [PATCH 11/24] Update noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- .../contracts/app/orderbook_contract/src/main.nr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index f2193c2cd7b7..cdfd1c5abdd8 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -94,7 +94,8 @@ pub contract Orderbook { .transfer_to_public(maker, context.this_address(), amount_in, nonce) .call(&mut context); - // Prepare a partial note that will get completed once the order is fulfilled. + // Prepare a partial note that will get completed once the order is fulfilled. Note that only we can complete the partial + // note. let maker_partial_note = Token::at(token_out).prepare_private_balance_increase(maker, maker).call(&mut context); From 5371330af8e4cf2bd16f22ba9020aa69a71d7054 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 08:03:17 +0000 Subject: [PATCH 12/24] improved comments --- .../contracts/app/orderbook_contract/src/main.nr | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index cdfd1c5abdd8..d94c0b232d4b 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -94,15 +94,14 @@ pub contract Orderbook { .transfer_to_public(maker, context.this_address(), amount_in, nonce) .call(&mut context); - // Prepare a partial note that will get completed once the order is fulfilled. Note that only we can complete the partial - // note. + // Prepare a partial note that will get completed once the order is fulfilled. Note that only the Orderbook + // contract can complete the partial note. let maker_partial_note = Token::at(token_out).prepare_private_balance_increase(maker, maker).call(&mut context); - // The partial note serves as a natural unique identifier for the order. While a maker could theoretically - // create duplicate order IDs by manipulating the randomness, this would be self-defeating - only one order - // with a given ID can be fulfilled, meaning any duplicates would result in the maker's tokens being - // irretrievably locked in the public balance of this contract. + // We use the partial note's commitment as the order ID. Because partial notes emit a nullifier with their + // commitment when created they are unique, and so this guarantees that our order IDs are also unique without + // having to keep track of past ones. let order_id = maker_partial_note.to_field(); // Store the order in public storage and emit an event. From 6c5c001a6b04bb720f8644e61d3efa73928d9123 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 08:10:27 +0000 Subject: [PATCH 13/24] nuking unnecessary comment --- .../contracts/app/orderbook_contract/src/main.nr | 5 ----- 1 file changed, 5 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index d94c0b232d4b..e46c6e03b778 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -145,11 +145,6 @@ pub contract Orderbook { // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order). We use the `order_id` // itself as the nullifier because this contract does not work with notes and hence there is no risk of // colliding with a real note nullifier. - // - // It's worth noting that since nullifier information is public, a malicious actor could potentially frontrun - // the taker's transaction. While this vulnerability exists in this simplified demonstration, a production - // implementation would typically employ a privileged matchmaker service to securely pair makers and takers, - // thereby ensuring order execution integrity. context.push_nullifier(order_id); // Enqueue the fulfillment to finalize both partial notes From 46eb3f3566e90af67eb21184ff58a92753c4fc9a Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 08:29:25 +0000 Subject: [PATCH 14/24] Returning whether an order has been fulfilled from get_order --- .../contracts/app/orderbook_contract/src/main.nr | 9 +++++++-- yarn-project/end-to-end/src/e2e_orderbook.test.ts | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index e46c6e03b778..19bfd146c0f4 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -39,6 +39,7 @@ pub contract Orderbook { functions::{initializer, internal, private, public, utility}, storage::storage, }, + oracle::notes::check_nullifier_exists, prelude::{AztecAddress, Map, PublicImmutable}, protocol_types::traits::{FromField, Serialize, ToField}, unencrypted_logs::unencrypted_event_emission::encode_event, @@ -186,8 +187,12 @@ pub contract Orderbook { OrderFulfilled { order_id }.emit(encode_event(&mut context)); } + /// Returns the order and whether it has been fulfilled. #[utility] - unconstrained fn get_order(order_id: Field) -> pub Order { - storage.orders.at(order_id).read() + unconstrained fn get_order(order_id: Field) -> pub (Order, bool) { + let order = storage.orders.at(order_id).read(); + let is_fulfilled = check_nullifier_exists(order_id); + + (order, is_fulfilled) } } diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index d5700370167a..218790a6b5cc 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -81,10 +81,11 @@ describe('Orderbook', () => { orderId = orderCreatedEvents[0].order_id; // Get order from orderbook and verify details - const order = await orderbook.methods.get_order(orderId).simulate(); + const [order, isFulfilled] = await orderbook.methods.get_order(orderId).simulate(); expect(order.amount_in).toEqual(amountIn); expect(order.amount_out).toEqual(amountOut); - expect(order.token_in_is_zero).toBeTruthy(); + expect(order.token_in_is_zero).toBeTrue(); + expect(isFulfilled).toBeFalse(); // At this point, amountIn of token0 should be transferred to the public balance of the orderbook and maker // should have 0. @@ -134,6 +135,10 @@ describe('Orderbook', () => { expect(takerBalances0).toEqual(amountIn); // Full taker token 1 balance should be transferred to maker and hence taker should have 0 expect(takerBalances1).toEqual(0n); + + // Verify that the order is fulfilled + const [_, isFulfilled] = await orderbook.methods.get_order(orderId).simulate(); + expect(isFulfilled).toBeTrue(); }); }); }); From 3d52b57df7edc7329e8bfe885add146cd3c7396a Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 08:36:17 +0000 Subject: [PATCH 15/24] linking issue --- yarn-project/end-to-end/src/e2e_orderbook.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index 218790a6b5cc..ee7e6a3c0701 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -9,9 +9,9 @@ import { setup } from './fixtures/utils.js'; const TIMEOUT = 120_000; -// We test only a happy path here since this is just a demonstration of the orderbook contract and writing here more -// thorough tests seems unnecessary. Also hopefully we will migrate this to TXE eventually. Didn't write it in TXE -// now as there is no way to obtain public events and all of TXE will be rewritten soon. +// TODO(#14525): Write thorough Orderbook tests. Currently we test only a happy path here because we will migrate these +// tests to TXE once TXE 2.0 is ready. Didn't write it in TXE now as there is no way to obtain public events and all of +// TXE will be rewritten soon. describe('Orderbook', () => { jest.setTimeout(TIMEOUT); From af2da1da2eca7e9e5b8bcd6f9a31f2f2aa593937 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 08:57:27 +0000 Subject: [PATCH 16/24] better token names --- .../app/orderbook_contract/src/config.nr | 16 +++--- .../app/orderbook_contract/src/main.nr | 54 +++++++++---------- .../app/orderbook_contract/src/order.nr | 27 +++++----- .../end-to-end/src/e2e_orderbook.test.ts | 34 ++++++------ 4 files changed, 66 insertions(+), 65 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index 51f924816a73..302065d56c1c 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -17,18 +17,18 @@ impl Config { pub fn validate_input_tokens_and_get_direction( self, - token_in: AztecAddress, - token_out: AztecAddress, + bid_token: AztecAddress, + ask_token: AztecAddress, ) -> bool { - assert((token_in == self.token0) | (token_in == self.token1), "TOKEN_IN_IS_INVALID"); - assert((token_out == self.token0) | (token_out == self.token1), "TOKEN_OUT_IS_INVALID"); - assert(token_in != token_out, "SAME_TOKEN_TRADE"); + assert((bid_token == self.token0) | (bid_token == self.token1), "BID_TOKEN_IS_INVALID"); + assert((ask_token == self.token0) | (ask_token == self.token1), "ASK_TOKEN_IS_INVALID"); + assert(bid_token != ask_token, "SAME_TOKEN_TRADE"); - token_in == self.token0 + bid_token == self.token0 } - pub fn get_tokens(self, token_in_is_zero: bool) -> (AztecAddress, AztecAddress) { - if token_in_is_zero { + pub fn get_tokens(self, bid_token_is_zero: bool) -> (AztecAddress, AztecAddress) { + if bid_token_is_zero { (self.token0, self.token1) } else { (self.token1, self.token0) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 19bfd146c0f4..ef0e548d1591 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -77,28 +77,28 @@ pub contract Orderbook { /// The maker specifies the tokens and amounts they want to trade #[private] fn create_order( - token_in: AztecAddress, - token_out: AztecAddress, - amount_in: u128, - amount_out: u128, + bid_token: AztecAddress, + ask_token: AztecAddress, + bid_amount: u128, + ask_amount: u128, nonce: Field, ) -> Field { let config = storage.config.read(); // Create the order (this validates the input tokens and amounts). - let order = Order::new(config, amount_in, amount_out, token_in, token_out); + let order = Order::new(config, bid_amount, ask_amount, bid_token, ask_token); let maker = context.msg_sender(); // Transfer tokens from maker to the public balance of this contract. - Token::at(token_in) - .transfer_to_public(maker, context.this_address(), amount_in, nonce) + Token::at(bid_token) + .transfer_to_public(maker, context.this_address(), bid_amount, nonce) .call(&mut context); // Prepare a partial note that will get completed once the order is fulfilled. Note that only the Orderbook // contract can complete the partial note. let maker_partial_note = - Token::at(token_out).prepare_private_balance_increase(maker, maker).call(&mut context); + Token::at(ask_token).prepare_private_balance_increase(maker, maker).call(&mut context); // We use the partial note's commitment as the order ID. Because partial notes emit a nullifier with their // commitment when created they are unique, and so this guarantees that our order IDs are also unique without @@ -127,19 +127,19 @@ pub contract Orderbook { let order = storage.orders.at(order_id).read(); let taker = context.msg_sender(); - // Determine which tokens are being exchanged based on token_in_is_zero flag - let (token_in, token_out) = config.get_tokens(order.token_in_is_zero); + // Determine which tokens are being exchanged based on bid_token_is_zero flag + let (bid_token, ask_token) = config.get_tokens(order.bid_token_is_zero); // TODO(#14362): Once we allow partial note completion in private we can skip the transfer to public here and // just finalize the maker's partial note straight away here. - // Transfer the amount_out from taker to the contract - Token::at(token_out) - .transfer_to_public(taker, context.this_address(), order.amount_out, nonce) + // Transfer the ask_amount from taker to the contract + Token::at(ask_token) + .transfer_to_public(taker, context.this_address(), order.ask_amount, nonce) .call(&mut context); - // Prepare partial note for taker to receive token_in + // Prepare partial note for taker to receive bid_token let taker_partial_note = - Token::at(token_in).prepare_private_balance_increase(taker, taker).call(&mut context); + Token::at(bid_token).prepare_private_balance_increase(taker, taker).call(&mut context); // Nullify the order such that it cannot be fulfilled again. We emit a nullifier instead of deleting the order // from public storage because we get no refund for resetting public storage to zero and just emitting @@ -153,10 +153,10 @@ pub contract Orderbook { ._fulfill_order( order_id, taker_partial_note, - token_in, - token_out, - order.amount_in, - order.amount_out, + bid_token, + ask_token, + order.bid_amount, + order.ask_amount, ) .enqueue(&mut context); } @@ -166,21 +166,21 @@ pub contract Orderbook { fn _fulfill_order( order_id: Field, taker_partial_note: PartialUintNote, - token_in: AztecAddress, - token_out: AztecAddress, - amount_in: u128, - amount_out: u128, + bid_token: AztecAddress, + ask_token: AztecAddress, + bid_amount: u128, + ask_amount: u128, ) { // The `order_id` is a serialized form of the maker's partial note. let maker_partial_note = PartialUintNote::from_field(order_id); - // Finalize transfer of amount_out of token_out to maker - Token::at(token_out).finalize_transfer_to_private(amount_out, maker_partial_note).call( + // Finalize transfer of ask_amount of ask_token to maker + Token::at(ask_token).finalize_transfer_to_private(ask_amount, maker_partial_note).call( &mut context, ); - // Finalize transfer of of amount_in of token_in to taker - Token::at(token_in).finalize_transfer_to_private(amount_in, taker_partial_note).call( + // Finalize transfer of bid_amount of bid_token to taker + Token::at(bid_token).finalize_transfer_to_private(bid_amount, taker_partial_note).call( &mut context, ); diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr index 600944a8370e..9ce4b29a362a 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr @@ -5,27 +5,28 @@ use aztec::{prelude::AztecAddress, protocol_types::traits::{Deserialize, Packabl // and save on public storage costs. #[derive(Deserialize, Eq, Packable, Serialize)] pub struct Order { - // Amount of input tokens - pub amount_in: u128, - // Amount of output tokens - pub amount_out: u128, + // Amount of bid tokens + pub bid_amount: u128, + // Amount of ask tokens + pub ask_amount: u128, // Whether the order is from token0 to token1 or from token1 to token0 - pub token_in_is_zero: bool, + pub bid_token_is_zero: bool, } impl Order { pub fn new( config: Config, - amount_in: u128, - amount_out: u128, - token_in: AztecAddress, - token_out: AztecAddress, + bid_amount: u128, + ask_amount: u128, + bid_token: AztecAddress, + ask_token: AztecAddress, ) -> Self { - assert(amount_in > 0 as u128, "ZERO_AMOUNT_IN"); - assert(amount_out > 0 as u128, "ZERO_AMOUNT_OUT"); + assert(bid_amount > 0 as u128, "ZERO_BID_AMOUNT"); + assert(ask_amount > 0 as u128, "ZERO_ASK_AMOUNT"); - let token_in_is_zero = config.validate_input_tokens_and_get_direction(token_in, token_out); + let bid_token_is_zero = + config.validate_input_tokens_and_get_direction(bid_token, ask_token); - Self { amount_in, amount_out, token_in_is_zero } + Self { bid_amount, ask_amount, bid_token_is_zero } } } diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index ee7e6a3c0701..7cffeb7c51fe 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -28,8 +28,8 @@ describe('Orderbook', () => { let token1: TokenContract; let orderbook: OrderbookContract; - const amountIn = 1000n; - const amountOut = 2000n; + const bidAmount = 1000n; + const askAmount = 2000n; beforeAll(async () => { ({ @@ -45,8 +45,8 @@ describe('Orderbook', () => { orderbook = await OrderbookContract.deploy(adminWallet, token0.address, token1.address).send().deployed(); // Mint tokens to maker and taker - await mintTokensToPrivate(token0, adminWallet, maker.getAddress(), amountIn); - await mintTokensToPrivate(token1, adminWallet, taker.getAddress(), amountOut); + await mintTokensToPrivate(token0, adminWallet, maker.getAddress(), bidAmount); + await mintTokensToPrivate(token1, adminWallet, taker.getAddress(), askAmount); }); afterAll(() => teardown()); @@ -57,16 +57,16 @@ describe('Orderbook', () => { it('creates an order', async () => { const nonceForAuthwits = Fr.random(); - // Create authwit for maker to allow orderbook to transfer amountIn of token0 to itself + // Create authwit for maker to allow orderbook to transfer bidAmount of token0 to itself const makerAuthwit = await maker.createAuthWit({ caller: orderbook.address, - action: token0.methods.transfer_to_public(maker.getAddress(), orderbook.address, amountIn, nonceForAuthwits), + action: token0.methods.transfer_to_public(maker.getAddress(), orderbook.address, bidAmount, nonceForAuthwits), }); // Create order await orderbook .withWallet(maker) - .methods.create_order(token0.address, token1.address, amountIn, amountOut, nonceForAuthwits) + .methods.create_order(token0.address, token1.address, bidAmount, askAmount, nonceForAuthwits) .with({ authWitnesses: [makerAuthwit] }) .send() .wait(); @@ -82,26 +82,26 @@ describe('Orderbook', () => { // Get order from orderbook and verify details const [order, isFulfilled] = await orderbook.methods.get_order(orderId).simulate(); - expect(order.amount_in).toEqual(amountIn); - expect(order.amount_out).toEqual(amountOut); + expect(order.amount_in).toEqual(bidAmount); + expect(order.amount_out).toEqual(askAmount); expect(order.token_in_is_zero).toBeTrue(); expect(isFulfilled).toBeFalse(); - // At this point, amountIn of token0 should be transferred to the public balance of the orderbook and maker + // At this point, bidAmount of token0 should be transferred to the public balance of the orderbook and maker // should have 0. const orderbookBalances0 = await token0.withWallet(maker).methods.balance_of_public(orderbook.address).simulate(); const makerBalances0 = await token0.withWallet(maker).methods.balance_of_private(maker.getAddress()).simulate(); - expect(orderbookBalances0).toEqual(amountIn); + expect(orderbookBalances0).toEqual(bidAmount); expect(makerBalances0).toEqual(0n); }); it('fulfills an order', async () => { const nonceForAuthwits = Fr.random(); - // Create authwit for taker to allow orderbook to transfer amountOut of token1 to itself + // Create authwit for taker to allow orderbook to transfer askAmount of token1 to itself const takerAuthwit = await taker.createAuthWit({ caller: orderbook.address, - action: token1.methods.transfer_to_public(taker.getAddress(), orderbook.address, amountOut, nonceForAuthwits), + action: token1.methods.transfer_to_public(taker.getAddress(), orderbook.address, askAmount, nonceForAuthwits), }); // Fulfill order @@ -129,10 +129,10 @@ describe('Orderbook', () => { // Full maker token 0 balance should be transferred to taker and hence maker should have 0 expect(makerBalances0).toEqual(0n); - // amountOut of token1 should be transferred to maker - expect(makerBalances1).toEqual(amountOut); - // amountIn of token0 should be transferred to taker - expect(takerBalances0).toEqual(amountIn); + // askAmount of token1 should be transferred to maker + expect(makerBalances1).toEqual(askAmount); + // bidAmount of token0 should be transferred to taker + expect(takerBalances0).toEqual(bidAmount); // Full taker token 1 balance should be transferred to maker and hence taker should have 0 expect(takerBalances1).toEqual(0n); From 994a1143c7523ec649c7a6684022d5bbdcf636ce Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 09:08:13 +0000 Subject: [PATCH 17/24] Add unit tests for Config and Order modules --- .../app/orderbook_contract/src/config.nr | 81 +++++++++++++++++++ .../app/orderbook_contract/src/order.nr | 55 +++++++++++++ 2 files changed, 136 insertions(+) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index 302065d56c1c..5fc71b03cd2c 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -35,3 +35,84 @@ impl Config { } } } + +mod test { + use crate::config::Config; + use aztec::{prelude::AztecAddress, protocol_types::traits::FromField}; + + #[test] + unconstrained fn new_config_valid_inputs() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + let (got_token0, got_token1) = config.get_tokens(true); + assert(got_token0 == token0); + assert(got_token1 == token1); + } + + #[test(should_fail_with = "Tokens must be different")] + unconstrained fn new_config_same_tokens() { + let token = AztecAddress::from_field(1); + let _ = Config::new(token, token); + } + + #[test] + unconstrained fn validate_input_tokens_valid() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + // Test token0 to token1 direction + let is_zero = config.validate_input_tokens_and_get_direction(token0, token1); + assert(is_zero); + + // Test token1 to token0 direction + let is_zero = config.validate_input_tokens_and_get_direction(token1, token0); + assert(!is_zero); + } + + #[test(should_fail_with = "BID_TOKEN_IS_INVALID")] + unconstrained fn validate_input_tokens_invalid_bid() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let token2 = AztecAddress::from_field(3); + let config = Config::new(token0, token1); + + let _ = config.validate_input_tokens_and_get_direction(token2, token1); + } + + #[test(should_fail_with = "ASK_TOKEN_IS_INVALID")] + unconstrained fn validate_input_tokens_invalid_ask() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let token2 = AztecAddress::from_field(3); + let config = Config::new(token0, token1); + + let _ = config.validate_input_tokens_and_get_direction(token0, token2); + } + + #[test(should_fail_with = "SAME_TOKEN_TRADE")] + unconstrained fn validate_input_tokens_same_token() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + let _ = config.validate_input_tokens_and_get_direction(token0, token0); + } + + #[test] + unconstrained fn get_tokens_correct_order() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + let (bid, ask) = config.get_tokens(true); + assert(bid == token0); + assert(ask == token1); + + let (bid, ask) = config.get_tokens(false); + assert(bid == token1); + assert(ask == token0); + } +} diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr index 9ce4b29a362a..2ba36043d428 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr @@ -30,3 +30,58 @@ impl Order { Self { bid_amount, ask_amount, bid_token_is_zero } } } + +mod test { + use crate::{config::Config, order::Order}; + use aztec::{prelude::AztecAddress, protocol_types::traits::FromField}; + + #[test] + unconstrained fn new_order_valid_inputs() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + let bid_amount = 100; + let ask_amount = 200; + + // Test token0 to token1 direction + let order = Order::new(config, bid_amount, ask_amount, token0, token1); + assert(order.bid_amount == bid_amount); + assert(order.ask_amount == ask_amount); + assert(order.bid_token_is_zero == true); + + // Test token1 to token0 direction + let order = Order::new(config, bid_amount, ask_amount, token1, token0); + assert(order.bid_amount == bid_amount); + assert(order.ask_amount == ask_amount); + assert(order.bid_token_is_zero == false); + } + + #[test(should_fail_with = "ZERO_BID_AMOUNT")] + unconstrained fn new_order_zero_bid_amount() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + let _ = Order::new(config, 0, 100, token0, token1); + } + + #[test(should_fail_with = "ZERO_ASK_AMOUNT")] + unconstrained fn new_order_zero_ask_amount() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let config = Config::new(token0, token1); + + let _ = Order::new(config, 100, 0, token0, token1); + } + + #[test(should_fail_with = "BID_TOKEN_IS_INVALID")] + unconstrained fn new_order_invalid_tokens() { + let token0 = AztecAddress::from_field(1); + let token1 = AztecAddress::from_field(2); + let token2 = AztecAddress::from_field(3); + let config = Config::new(token0, token1); + + let _ = Order::new(config, 100, 100, token2, token1); + } +} From 3aa3fa5e6b62a4944d21e84b47baf702b4c74510 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 26 May 2025 10:44:07 +0000 Subject: [PATCH 18/24] test fix post rename --- yarn-project/end-to-end/src/e2e_orderbook.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_orderbook.test.ts b/yarn-project/end-to-end/src/e2e_orderbook.test.ts index 7cffeb7c51fe..8c717d1f1257 100644 --- a/yarn-project/end-to-end/src/e2e_orderbook.test.ts +++ b/yarn-project/end-to-end/src/e2e_orderbook.test.ts @@ -82,9 +82,9 @@ describe('Orderbook', () => { // Get order from orderbook and verify details const [order, isFulfilled] = await orderbook.methods.get_order(orderId).simulate(); - expect(order.amount_in).toEqual(bidAmount); - expect(order.amount_out).toEqual(askAmount); - expect(order.token_in_is_zero).toBeTrue(); + expect(order.bid_amount).toEqual(bidAmount); + expect(order.ask_amount).toEqual(askAmount); + expect(order.bid_token_is_zero).toBeTrue(); expect(isFulfilled).toBeFalse(); // At this point, bidAmount of token0 should be transferred to the public balance of the orderbook and maker @@ -95,6 +95,7 @@ describe('Orderbook', () => { expect(makerBalances0).toEqual(0n); }); + // Note that this test case depends on the previous one. it('fulfills an order', async () => { const nonceForAuthwits = Fr.random(); From b801c42f422f756661f2b373463c17e1055b1503 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 27 May 2025 13:55:04 +0000 Subject: [PATCH 19/24] Refactor token initialization in tests to use global variables for consistency and clarity --- .../app/orderbook_contract/src/config.nr | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index 5fc71b03cd2c..ddbe1f465899 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -40,10 +40,12 @@ mod test { use crate::config::Config; use aztec::{prelude::AztecAddress, protocol_types::traits::FromField}; + global token0: AztecAddress = AztecAddress::from_field(1); + global token1: AztecAddress = AztecAddress::from_field(2); + global token2: AztecAddress = AztecAddress::from_field(3); + #[test] unconstrained fn new_config_valid_inputs() { - let token0 = AztecAddress::from_field(1); - let token1 = AztecAddress::from_field(2); let config = Config::new(token0, token1); let (got_token0, got_token1) = config.get_tokens(true); @@ -53,14 +55,11 @@ mod test { #[test(should_fail_with = "Tokens must be different")] unconstrained fn new_config_same_tokens() { - let token = AztecAddress::from_field(1); - let _ = Config::new(token, token); + let _ = Config::new(token0, token0); } #[test] unconstrained fn validate_input_tokens_valid() { - let token0 = AztecAddress::from_field(1); - let token1 = AztecAddress::from_field(2); let config = Config::new(token0, token1); // Test token0 to token1 direction @@ -74,37 +73,24 @@ mod test { #[test(should_fail_with = "BID_TOKEN_IS_INVALID")] unconstrained fn validate_input_tokens_invalid_bid() { - let token0 = AztecAddress::from_field(1); - let token1 = AztecAddress::from_field(2); - let token2 = AztecAddress::from_field(3); let config = Config::new(token0, token1); - let _ = config.validate_input_tokens_and_get_direction(token2, token1); } #[test(should_fail_with = "ASK_TOKEN_IS_INVALID")] unconstrained fn validate_input_tokens_invalid_ask() { - let token0 = AztecAddress::from_field(1); - let token1 = AztecAddress::from_field(2); - let token2 = AztecAddress::from_field(3); let config = Config::new(token0, token1); - let _ = config.validate_input_tokens_and_get_direction(token0, token2); } #[test(should_fail_with = "SAME_TOKEN_TRADE")] unconstrained fn validate_input_tokens_same_token() { - let token0 = AztecAddress::from_field(1); - let token1 = AztecAddress::from_field(2); let config = Config::new(token0, token1); - let _ = config.validate_input_tokens_and_get_direction(token0, token0); } #[test] unconstrained fn get_tokens_correct_order() { - let token0 = AztecAddress::from_field(1); - let token1 = AztecAddress::from_field(2); let config = Config::new(token0, token1); let (bid, ask) = config.get_tokens(true); From 13b203b32badb582b7bd1bc685259d93f50605ea Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 27 May 2025 13:57:09 +0000 Subject: [PATCH 20/24] Add documentation for get_tokens method to clarify its return values based on bid_token_is_zero parameter --- .../contracts/app/orderbook_contract/src/config.nr | 1 + 1 file changed, 1 insertion(+) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index ddbe1f465899..3ee55b3c8f26 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -27,6 +27,7 @@ impl Config { bid_token == self.token0 } + /// Returns a tuple of (bid_token, ask_token) based on `bid_token_is_zero` param. pub fn get_tokens(self, bid_token_is_zero: bool) -> (AztecAddress, AztecAddress) { if bid_token_is_zero { (self.token0, self.token1) From 88fb65fca7414ad82434324dc9f7f26207c14781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Tue, 27 May 2025 15:58:52 +0200 Subject: [PATCH 21/24] Update noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- .../contracts/app/orderbook_contract/src/main.nr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index ef0e548d1591..88dacc29c234 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -100,9 +100,9 @@ pub contract Orderbook { let maker_partial_note = Token::at(ask_token).prepare_private_balance_increase(maker, maker).call(&mut context); - // We use the partial note's commitment as the order ID. Because partial notes emit a nullifier with their - // commitment when created they are unique, and so this guarantees that our order IDs are also unique without - // having to keep track of past ones. + // We use the partial note's as the order ID. Because partial notes emit a nullifier when created they are + // unique, and so this guarantees that our order IDs are also unique without having to keep track of past + // ones. let order_id = maker_partial_note.to_field(); // Store the order in public storage and emit an event. From 294c94f2db15b192404f015bb800fd4d14e81141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Tue, 27 May 2025 16:00:40 +0200 Subject: [PATCH 22/24] Update noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- .../contracts/app/orderbook_contract/src/config.nr | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index 3ee55b3c8f26..30a7d77aaf53 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -47,11 +47,7 @@ mod test { #[test] unconstrained fn new_config_valid_inputs() { - let config = Config::new(token0, token1); - - let (got_token0, got_token1) = config.get_tokens(true); - assert(got_token0 == token0); - assert(got_token1 == token1); + let _ = Config::new(token0, token1); } #[test(should_fail_with = "Tokens must be different")] From 65f6141031d37f818eb0010a0d4c60ccb9a656a1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 27 May 2025 14:04:32 +0000 Subject: [PATCH 23/24] Refactor get_tokens test to use validate_input_tokens_and_get_direction for improved clarity --- .../contracts/app/orderbook_contract/src/config.nr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr index 30a7d77aaf53..5ce897d434b8 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr @@ -90,11 +90,13 @@ mod test { unconstrained fn get_tokens_correct_order() { let config = Config::new(token0, token1); - let (bid, ask) = config.get_tokens(true); + let is_zero = config.validate_input_tokens_and_get_direction(token0, token1); + let (bid, ask) = config.get_tokens(is_zero); assert(bid == token0); assert(ask == token1); - let (bid, ask) = config.get_tokens(false); + let is_zero = config.validate_input_tokens_and_get_direction(token1, token0); + let (bid, ask) = config.get_tokens(is_zero); assert(bid == token1); assert(ask == token0); } From 50e2bdf655d2312d1c97ae69b6e4f6e71146861e Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 28 May 2025 07:00:08 +0000 Subject: [PATCH 24/24] comment --- .../noir-contracts/contracts/app/orderbook_contract/src/main.nr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 88dacc29c234..6616f855e7ec 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -114,6 +114,8 @@ pub contract Orderbook { #[public] #[internal] fn _create_order(order_id: Field, order: Order) { + // Note that PublicImmutable can be initialized only once so this is a secondary check that the order is + // unique. storage.orders.at(order_id).initialize(order); OrderCreated { order_id }.emit(encode_event(&mut context));