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(examples): creating an order via smart-contract wallet #185

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

shoom3301
Copy link
Contributor

@shoom3301 shoom3301 commented Dec 8, 2023

Example for creating pre-signed order.
This example will be used for the docs: cowprotocol/docs#9

Demo:

presign-demo.mov

@shoom3301 shoom3301 requested review from mfw78 and a team December 8, 2023 11:13
@shoom3301 shoom3301 self-assigned this Dec 8, 2023
Copy link

github-actions bot commented Dec 8, 2023

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request

@coveralls
Copy link
Collaborator

coveralls commented Dec 8, 2023

Coverage Status

coverage: 79.051%. remained the same
when pulling 9ded2b1 on feat/sc-wallet-order
into 7ef442e on main.

Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
https 1.0.0 None +0 331 B hardus
util 0.12.5 environment +8 244 kB goto-bus-stop
assert 2.1.0 None +10 336 kB ljharb
buffer 6.0.3 None +0 91.3 kB feross
url 0.11.3 None +4 402 kB ljharb

Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Also great test to later try to remove those dev-dependencies

"crypto": "npm:crypto-browserify@3.12.0",
"stream": "npm:stream-browserify@3.0.0",
"url": "npm:url@0.11.3",
"util": "npm:util@0.12.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all the libs u mentioned we depend on?
This is what you plan to improve? is the final outcome that we don't need to add any of these dev dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was talking about them.
But I've discovered that they actually came from @safe-global/api-kit and @safe-global/protocol-kit.
So, there is no problem with SDK itself.
I've tried to up Safe libs versions but haven't succeed, because they are using ethers@6.x.x.
In conclusion:

  • cow-sdk is good, no issues there
  • Safe libs with ethers@5.x.x have issues with node lbs
  • we can't update Safe libs because new versions are based on ethers@6.x.x

Copy link
Contributor

@anxolin anxolin Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ups!, I see 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it prefixed with npm: ?

@shoom3301 shoom3301 merged commit 88653a8 into main Dec 12, 2023
8 of 9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2023
@alfetopito alfetopito deleted the feat/sc-wallet-order branch December 12, 2023 10:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants