Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: L2 aggregation for uniswap L1 swap #1796

Closed
wants to merge 5 commits into from
Closed

Conversation

LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Aug 24, 2023

This PR fixes #879 and supports aggregation using public functions where the user can later claim funds in private. It is not full aztec connect style, but almost.

Bridged assets have too high precision for noir to handle 😬 Both dai and weth are 1e18 but we want the u120 to do math on them which is running into trouble when multiplying. To avoid the issue, i'm reducing the precision of input assets by 1e9 in the share computation.

BROKEN

This PR was broken after changes to how the unshielding work in underlying token implementation. Earlier they were requiring only knowledge of the pre-image and ability to create the nullifier, now they also depend on the contract coming from a specific msg_sender. This makes the flow in both the lending and this contract broken since they were using this structure before 🤷. As alluded to in #1743 a standard should be made.

Since only some assets where updated, there are really two paths to this case.

  1. Simply use another token implementation and don't think much about it
  2. Fix the vulnerable tokens, private-"approvals" and then these contracts.

Consider the discussion that @spalladino and I had in https://discourse.aztec.network/t/account-guardians-eip1271-and-recursive-proofs/675

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

@LHerskind LHerskind marked this pull request as ready for review August 25, 2023 11:46
@LHerskind LHerskind marked this pull request as draft August 29, 2023 19:13
@LHerskind LHerskind added the S-blocked Status: Blocked label Sep 1, 2023
@ludamad
Copy link
Collaborator

ludamad commented Oct 11, 2023

Closed as stale, reopen as needed.

@ludamad ludamad closed this Oct 11, 2023
@ludamad ludamad deleted the lh/879-uniswap branch August 22, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: Mini Aztec Connect With Uniswap
2 participants