Merged
Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
2 times, most recently
from
5b3fdc6 to
c9c8a76
Compare
benesjan
changed the base branch from
master
to
05-22-refactor_separating_getlogsbytags_to_public_and_private_versions
benesjan
changed the base branch from
05-22-refactor_separating_getlogsbytags_to_public_and_private_versions
to
next
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
from
c9c8a76 to
5089fa3
Compare
benesjan
changed the base branch from
master
to
05-22-refactor_separating_getlogsbytags_to_public_and_private_versions
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
from
5089fa3 to
962accd
Compare
benesjan
commented
May 23, 2025
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Show resolved
Hide resolved
benesjan
commented
May 23, 2025
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Show resolved
Hide resolved
nventuro
reviewed
May 23, 2025
Contributor
nventuro
left a comment
nventuro
left a comment
There was a problem hiding this comment.
This is so much nicer than I thought it'd be. Are we actually doing a good job?
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/order.nr
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Outdated
Show resolved
Hide resolved
benesjan
force-pushed
the
05-22-refactor_separating_getlogsbytags_to_public_and_private_versions
branch
from
1faf365 to
2d318b3
Compare
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
2 times, most recently
from
e339b2a to
807baf1
Compare
This was referenced May 26, 2025
benesjan
commented
May 26, 2025
| #[utility] | ||
| 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); |
Contributor
Author
There was a problem hiding this comment.
We have nullifier_exists function on public context but not on utility and private contexts. Shall we add it?
Contributor
There was a problem hiding this comment.
Unsure. The way we do this in private is with the prove_nullifier_inclusion fn, which expects a full block header. It'd not touch these too much until we use them more, or get external feedback.
wei3erHase
reviewed
May 26, 2025
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Show resolved
Hide resolved
wei3erHase
reviewed
May 26, 2025
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Outdated
Show resolved
Hide resolved
nventuro
approved these changes
May 27, 2025
Contributor
nventuro
left a comment
nventuro
left a comment
There was a problem hiding this comment.
such smart
much nice
wow
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/config.nr
Outdated
Show resolved
Hide resolved
| #[utility] | ||
| 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); |
Contributor
There was a problem hiding this comment.
Unsure. The way we do this in private is with the prove_nullifier_inclusion fn, which expects a full block header. It'd not touch these too much until we use them more, or get external feedback.
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr
Show resolved
Hide resolved
benesjan
force-pushed
the
05-22-refactor_separating_getlogsbytags_to_public_and_private_versions
branch
from
aff6028 to
0e0996d
Compare
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
from
0610d9f to
6c8ce5d
Compare
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
from
fca2253 to
989ed05
Compare
benesjan
changed the base branch from
05-22-refactor_separating_getlogsbytags_to_public_and_private_versions
to
graphite-base/14464
Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
…src/main.nr Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
…nsistency and clarity
…based on bid_token_is_zero parameter
…src/main.nr Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
…src/config.nr Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
…on for improved clarity
benesjan
force-pushed
the
05-22-feat_simple_orderbook_dex
branch
from
3b5017b to
65f6141
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implemented an orderbook DEX as we need it to demonstrate partial note completion in private flow.
Pointed this to
05-22-refactor_separating_getlogsbytags_to_public_and_private_versionsbranch as to avoid getting unwanted collisions of Orderbook's public logs with token's partial note completion logs.