Lido offchain plugin p3, Uniswap plugin #439

luketchang · 2023-09-08T12:44:43Z

Motivation

Add uniswap plugin.

Solution

Change op request builder interface to take optional config not optional teller address (we need handler address for uniswap output address and other stuff)
Add uniswap plugin

Proof

N/A (will test once UI is ready)
PR so interface still builds is here: https://github.com/nocturne-xyz/interface/pull/128

PR Checklist

added tests
updated documentation
added changeset if necessary
tested in dev/testnet
tested site with snap (we haven't automated this yet)
re-built & tested circuits if any of them changed
updated contracts storage layout (if contracts were updated)

changeset-bot · 2023-09-08T12:44:47Z

🦋 Changeset detected

Latest commit: 0c75bac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages

Name	Type
@nocturne-xyz/core	Major
@nocturne-xyz/op-request-plugins	Minor
@nocturne-xyz/deploy	Patch
@nocturne-xyz/e2e-tests	Patch
@nocturne-xyz/frontend-sdk	Patch
@nocturne-xyz/idb-kv-store	Patch
@nocturne-xyz/local-prover	Patch
@nocturne-xyz/persistent-log	Patch
@nocturne-xyz/bundler	Patch
@nocturne-xyz/deposit-screener	Patch
@nocturne-xyz/insertion-writer	Patch
@nocturne-xyz/subtree-updater	Patch
@nocturne-xyz/test-actor	Patch
@nocturne-xyz/snap	Patch
@nocturne-xyz/offchain-utils	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

luketchang · 2023-09-08T12:44:54Z

Current dependencies on/for this PR:

main
- PR Lido offchain plugin p3, Uniswap plugin #439 👈
  - PR fix: plugins propagate actual refund amounts for op.trackedAssets #441
    - PR feat: deploy script conditionally deploys wsteth adapter + add goerli config #446

This comment was auto-generated by Graphite.

luketchang · 2023-09-10T15:05:24Z

package.json

+  "resolutions": {
+    "@uniswap/sdk-core": "^4.0.3"
+  },


For the life of me could not resolve some versioning issues btwn uniswap router-sdk -> sdk-core and sdk-core. This was the only thing that worked to fix it

ok - do you know if it still works if you install the plugin package separately? If not, nw, we can publish an alpha for it and try installing it into interface later when we "acceptance test" it anyways.

Yeah installing directly into plugins package didn't work

Sladuca

looks ok aside from unit tests failing and previously-discussed lack of tests - just had a few small questions and one nit change that may or may not apply

Sladuca · 2023-09-10T18:49:11Z

package.json

+  "resolutions": {
+    "@uniswap/sdk-core": "^4.0.3"
+  },


ok - do you know if it still works if you install the plugin package separately? If not, nw, we can publish an alpha for it and try installing it into interface later when we "acceptance test" it anyways.

Sladuca · 2023-09-10T18:52:49Z

packages/core/src/primitives/types.ts

+  | {
+      type: "Action";
+      actionType: "UniswapV3 Swap";
+      tokenIn: Address;
+      inAmount: bigint;
+      tokenOut: Address;


This is ok for now, but we're going to need to come back and generalize this. I think the sanest way to deal with probably going to end up requiring plugins to format metadata.

Sladuca · 2023-09-10T18:56:37Z

packages/core/src/operationRequest/builder.ts

@@ -109,14 +114,14 @@ export type JoinSplitsAndPaymentsForAsset = [
 export function newOpRequestBuilder(
  provider: ethers.providers.Provider,
  chainId: bigint,
-  tellerContract?: Address // for testing purposes in case there is no config for test network


Sladuca · 2023-09-10T19:08:29Z

packages/op-request-plugins/src/UniswapV3Plugin.ts

+              resolve({
+                unwraps: [unwrap],
+                confidentialPayments: [],
+                actions: [approveAction, swapAction], // enqueue approve + swap


Do we need to revoke approval and/or limit the approval? Is the thinking here that it's kind of pointless because handler is shared?

Need to check on this, I don't think I understand permit2 very well rn

Yeah so you have one-time approve max uint256 for permit2 approvals. See uniswap article here for little bit more of explanation. What's above basically just does max approval if needed, can serve for any user later on to
https://blog.uniswap.org/permit2-integration-guide

Nvm I changed to swap router which does traditional token approval to protocol, just doing max uint256 because anyone can call handler.approve

Sladuca · 2023-09-10T19:10:00Z

packages/op-request-plugins/src/UniswapV3Plugin.ts

+        }
+      );
+
+      this.pluginFn(prom);


does pluginFn chain? If so we can just do return this.pluginFn(prom)

Yeah it does return this but doing return this.pluginFn(...) throws error because I think it doesn't view EInner as compatible with this?

yeah this is what I was referring to re: this getting bound to the wrong thing.

https://www.notion.so/nocturnelabs/Fix-this-pluginFn-return-type-issue-63d976fb3b7c43afa51cf6104be0dc80?pvs=4

…SA for reentrancy

luketchang commented Sep 10, 2023

View reviewed changes

luketchang marked this pull request as ready for review September 10, 2023 18:10

luketchang requested a review from Sladuca as a code owner September 10, 2023 18:10

Sladuca approved these changes Sep 10, 2023

View reviewed changes

luketchang force-pushed the luke/lido-offchain-3 branch from b884d12 to bb2cb5f Compare September 12, 2023 16:05

luketchang enabled auto-merge (squash) September 12, 2023 16:08

luketchang disabled auto-merge September 12, 2023 16:19

luketchang enabled auto-merge (squash) September 12, 2023 16:42

luketchang disabled auto-merge September 12, 2023 16:45

luketchang mentioned this pull request Sep 12, 2023

fix: plugins propagate actual refund amounts for op.trackedAssets #441

Merged

7 tasks

luketchang force-pushed the main branch from 63a9243 to 9098e2c Compare September 12, 2023 18:58

luketchang force-pushed the luke/lido-offchain-3 branch from e833d12 to c486f04 Compare September 12, 2023 19:08

luketchang mentioned this pull request Sep 12, 2023

feat: deploy script conditionally deploys wsteth adapter + add goerli config #446

Merged

7 tasks

luketchang force-pushed the luke/lido-offchain-3 branch from c486f04 to 64f15ce Compare September 13, 2023 18:58

luketchang added 12 commits September 14, 2023 08:05

change optional builder param to config not teller addr

b612ae3

add uniswap plugin draft

a8208b1

nits

c31e183

fix uniswap plugin to use universal router

5938399

changesets

ef2b871

try/catch in uniswap plugin, turn off async executor lint to warn

41a2033

fix core unit tests

8adb812

fix plugin unit test to take config not teller addr

90bd7d4

fix: uni plugin calls swap router not universal router due to lowe r …

5d5f8a0

…SA for reentrancy

plugins pull addrs from config

780b496

fix op request wsteth plugin test

b700342

yarn i

0c75bac

luketchang force-pushed the luke/lido-offchain-3 branch from cb943b0 to 0c75bac Compare September 14, 2023 12:20

luketchang merged commit 0cb20e3 into main Sep 14, 2023
3 checks passed

luketchang deleted the luke/lido-offchain-3 branch September 14, 2023 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lido offchain plugin p3, Uniswap plugin #439

Lido offchain plugin p3, Uniswap plugin #439

luketchang commented Sep 8, 2023 •

edited

Loading

changeset-bot bot commented Sep 8, 2023 •

edited

Loading

luketchang commented Sep 8, 2023 •

edited

Loading

luketchang Sep 10, 2023

Sladuca Sep 10, 2023 •

edited

Loading

luketchang Sep 11, 2023

Sladuca left a comment •

edited

Loading

Sladuca Sep 10, 2023 •

edited

Loading

Sladuca Sep 10, 2023

Sladuca Sep 10, 2023

Sladuca Sep 10, 2023

luketchang Sep 11, 2023

luketchang Sep 12, 2023

luketchang Sep 12, 2023

Sladuca Sep 10, 2023

luketchang Sep 12, 2023 •

edited

Loading

Sladuca Sep 12, 2023

luketchang Sep 12, 2023

Lido offchain plugin p3, Uniswap plugin #439

Lido offchain plugin p3, Uniswap plugin #439

Conversation

luketchang commented Sep 8, 2023 • edited Loading

Motivation

Solution

Proof

PR Checklist

changeset-bot bot commented Sep 8, 2023 • edited Loading

🦋 Changeset detected

luketchang commented Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Sladuca Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sladuca left a comment • edited Loading

Choose a reason for hiding this comment

Sladuca Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luketchang Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luketchang commented Sep 8, 2023 •

edited

Loading

changeset-bot bot commented Sep 8, 2023 •

edited

Loading

luketchang commented Sep 8, 2023 •

edited

Loading

Sladuca Sep 10, 2023 •

edited

Loading

Sladuca left a comment •

edited

Loading

Sladuca Sep 10, 2023 •

edited

Loading

luketchang Sep 12, 2023 •

edited

Loading