Fix bugs in ECAdd bloq #1489
Fix bugs in ECAdd bloq #1489
Conversation
Co-authored-by: Anurudh Peduri <[email protected]>
|
@mpharrigan Here are the bug fixes to the ECAdd circuit that the paper didn't consider. They are thoroughly tested with the existing unit tests. |
|
Nice! The issue and PR description describes the modifications and why they were needed -- but I suspect only the most dedicated users will find them! Do you want to put a description of the modifications into the library somewhere? Ideally this would be in the docstrings of the public bloq classes so they'd get rendered into the docs |
|
an alternative would be to have a dedicated jupyter notebook written as a mini-paper that goes through all the crypto bloqs and includes a section on the modifications. This would be more work but this is the sort of thing we can slap a byline on and get a zenodo doi to make it citable |
| aby_arr = np.concatenate([bb.split(a), bb.split(b), bb.split(y)]) | ||
| aby_arr, f2 = bb.add(MultiControlX(cvs=[0] * 3 * self.n), controls=aby_arr, target=f2) | ||
| aby_arr = np.split(aby_arr, 3) | ||
| a = bb.join(aby_arr[0], dtype=QMontgomeryUInt(self.n)) | ||
| b = bb.join(aby_arr[1], dtype=QMontgomeryUInt(self.n)) | ||
| y = bb.join(aby_arr[2], dtype=QMontgomeryUInt(self.n)) | ||
|
|
||
| xyb_arr = np.concatenate([bb.split(x), bb.split(y), bb.split(b)]) | ||
| xyb_arr, f2 = bb.add(MultiControlX(cvs=[0] * 3 * self.n), controls=xyb_arr, target=f2) | ||
| xyb_arr = np.split(xyb_arr, 3) | ||
| x = bb.join(xyb_arr[0], dtype=QMontgomeryUInt(self.n)) | ||
| y = bb.join(xyb_arr[1], dtype=QMontgomeryUInt(self.n)) | ||
| b = bb.join(xyb_arr[2], dtype=QMontgomeryUInt(self.n)) |
There was a problem hiding this comment.
I think you can replace this with the default controlled to avoid manual splits and joins:
mcx = XGate().controlled(CtrlSpec(qdtypes=QMontgomeryUInt(self.n), cvs=[0, 0, 0]))
[a, b, y], f2 = bb.add(mcx, ctrl=[a, b, y], q=f2)
[x, y, b], f2 = bb.add(mcx, ctrl=[x, y, b], q=f2)Though I suspect the types may not be propagated correctly yet. In case you try the above suggestion and it fails, could you please open an issue?
p.s. this would also enable decomposing for symbolic self.n which would be an added benefit.
The circuit described in the paper had 4 bugs (#1461) which caused (with edge case inputs) flags f1, f2, f4 and lambda to be freed (considered = 0 in the paper) while having values > 0.
Bugs:
1 - Step 2: lam = lam_r undoes f1 when it should only happen if lam is set to lam_r (very rare case)
BUGFIX:
2 - Step 5: free dirty reg lam when a,b=x,y
BUGFIX:
3 - Step 6: free dirty reg when x,y = 0,0 and b = y = 0, but x != 0: f2 never gets flipped back
BUGFIX:
4 - Step 6: when p1 = p2 and f4 not set f4 gets flipped on
BUGFIX: