feat: add common functions from emp-wasm #7
Conversation
There was a problem hiding this comment.
I'm curious what source(s) this version of AsyncQueue is based on 🤔. I'm happy that you made updates, just curious.
| return Promise.reject(new Error('Queue is closed')); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
We need a stream method in here, similar to
https://github.com/privacy-scaling-explorations/mpc-hello/blob/7470c90/client-client/src/AsyncQueue.ts#L41
| * @returns A Promise resolving with the popped item. | ||
| * @throws {Error} If the queue is closed. | ||
| */ | ||
| pop(): Promise<T> { |
There was a problem hiding this comment.
I like that you called this pop instead of shift (was your idea or did you get it from a version that was like that? 🤔). This method will need to be changed to facilitate stream but please keep the name pop (and don't add shift).
| * @param channel - The channel identifier (typically 'a' or 'b') | ||
| * @returns An AsyncQueue instance for the specified communication channel | ||
| */ | ||
| get(from: number | string, to: number | string, channel: string): AsyncQueue<T> { |
There was a problem hiding this comment.
The shape of this method is specific to the channels concept in emp-wasm, which is bad. mpc-framework-common should avoid particulars of its dependencies. Please make it get(key: string): AsyncQueue<T> instead.
| * @param channel - The channel identifier ('a' or 'b') | ||
| * @returns A BufferQueue instance for the specified communication channel | ||
| */ | ||
| get(from: number | string, to: number | string, channel: 'a' | 'b'): BufferQueue { |
There was a problem hiding this comment.
Similar to previous comment (see "shape of this method").
There was a problem hiding this comment.
Please delete this and add .DS_Store to .gitignore.
What is this PR doing?
adding common utilities from
emp-wasmtompc-framework-common:These utilities are general-purpose. It aims reduces duplication and establish a single source of truth.
How can these changes be manually tested?
Does this PR resolve or contribute to any issues?
Yes, PR related to MPC Framework issue - privacy-scaling-explorations/mpc-framework#10