Conversation
benesjan
force-pushed
the
06-11-feat_closedsetorderbook
branch
2 times, most recently
from
8caa285 to
ddc67dc
Compare
benesjan
force-pushed
the
06-11-feat_closedsetorderbook
branch
from
ddc67dc to
4de0321
Compare
benesjan
force-pushed
the
06-11-feat_closedsetorderbook
branch
4 times, most recently
from
74e6300 to
31ba402
Compare
benesjan
force-pushed
the
06-11-feat_closedsetorderbook
branch
from
31ba402 to
586ca0a
Compare
benesjan
changed the base branch from
graphite-base/14977
to
07-10-fix_bug_in_pxe_getnotes
benesjan
force-pushed
the
06-11-feat_closedsetorderbook
branch
from
6d37654 to
0fc9fb3
Compare
| ) { | ||
| // TODO: Why don't we have a way to directly initialize SharedMutable when it's created? If this is done | ||
| // in the constructor then it's safe as we cannot interact with an undeployed contract that has initializer. | ||
| // Waiting for delay to pass here is quite annoying. |
There was a problem hiding this comment.
Curious if reviewer thinks we would want to support something like this. When a contract needs to be initialized and the value is scheduled in #[initializer] then directly activating should be safe.
| .transfer_in_private(maker, context.this_address(), bid_amount, authwit_nonce) | ||
| .call(&mut context); | ||
|
|
||
| // This contract is the "nullificator" of the order note meaning that this contract's nullifier key will be |
There was a problem hiding this comment.
Can rename this to "nelly" if we want to stick to that name instead.
| // We get and nullify the order. We are working around PrivateImmutable here because the PrivateImmutable | ||
| // getter does not allow us to then destroy the note. This is of course intentional as the note is supposed | ||
| // to be immutable. But we want to nullify the note as it's valuable to communicate order fulfillment that way. | ||
| // TODO: Rework this. An option is to not use any state variable and instead manually handle the notes. |
There was a problem hiding this comment.
I think this is quite interesting as here we essentially want PrivateImmutable + destroy. As in the only "modification" allowed is a destruction.
Anyway I would probably solve this by expanding PrivateMutable to have some kind of get and destroy function and use it here.
WDYT?
| out.value(), | ||
| prev, | ||
| index, | ||
| self.hints.sorted_kept_note_hash_indexes[index], |
There was a problem hiding this comment.
This was a fix done by Leila. See this discussion on slack for context.
benesjan
force-pushed
the
06-11-feat_closedsetorderbook
branch
from
cfba7d0 to
31188d7
Compare
|
Failed to get @nventuro to review this for half a year so there is no point in keeping it around. |
2 participants
Fixes #14917
Note for reviewer
I think this PR is now quite complete and the only significant thing missing is the implementation and usage of
ClosedSharedMutablehere (relevant comment). But anyway that should be done in a followup PR so I think once the TODOs that are present in the code and that I want to get a feedback on are addressed I think this can be merged.