ethpmV3 support by njgheorghita · Pull Request #3247 · ConsenSys-archive/truffle

njgheorghita · 2020-08-10T23:27:43Z

Truffle support for the ethpmV3 spec!

Hey @gnidan and @eggplantzzz . Thanks for your help so far in discussing this. Here's a first pass at ethpmV3 x truffle. I'm sure there's plenty of areas where my javascript is smelly, please point them out anywhere you see them. After a review, I plan to run this through another thorough round of integration tests, & I have an update to the trufflesuite.com docs in the pipeline to reflect these changes.

packages/core/lib/commands/packages.js

packages/ethpm-v1/package.json

njgheorghita · 2020-08-11T00:23:13Z

packages/config/src/configDefaults.ts

+      ipfsProtocol: "https",
+      ipfsPort: "5001",
+      registry: {
+        address: "0x8011df4830b4f696cd81393997e5371b93338878",


I'm happy to deploy an un-authenticated registry for truffle to use as its default, unless you guys want to handle that. I'm 50/50 on whether it should be on mainnet (better reliability / authority / helps minimize spam packages) or on any testnet (much much cheaper / easier to start getting your hands dirty with publishing ethpm packages)

does this field respect ENS names?

Yep, though right now it only permits ens on mainnet. However, upon further investigation it looks like truffle supports ens on all testnets except kovan, so I updated it to support ens on these other testnets.

packages/ethpm-v1/README.md

njgheorghita · 2020-08-11T00:34:32Z

packages/ethpm-v3/lib/ethpm-v3.js

+      gasPrice: 100000000000,
+      data: encodedTxData
+    });
+    try {


It seems like there might be a better mechanism available somewhere in truffle that manages sending txs with a more user-friendly interface? Could you point me in the right direction...

packages/core/lib/commands/install.js

gnidan

Here's an initial review on the @truffle/core plumbing stuff

gnidan · 2020-08-11T15:31:11Z

packages/config/src/configDefaults.ts

+      ipfsProtocol: "https",
+      ipfsPort: "5001",
+      registry: {
+        address: "0x8011df4830b4f696cd81393997e5371b93338878",


does this field respect ENS names?

gnidan · 2020-08-11T15:31:28Z

packages/core/lib/commands/install.js

  builder: {},
  help: {
-    usage: "truffle install <package_name>[@<version>]",
+    usage: "truffle install <packageId> [--alias]",


Suggested change

usage: "truffle install <packageId> [--alias]",

usage: "truffle install <package-identifier> [--alias]",

packages/core/lib/commands/install.js

packages/core/lib/commands/packages.js

packages/core/lib/commands/publish.js

packages/core/lib/commands/packages.js

packages/core/lib/commands/install.js

packages/ethpm-v3/lib/utils.js

njgheorghita · 2020-10-06T14:45:05Z

@eggplantzzz Thanks for the review! I've changed or responded to your comments. Let me know if there's anything else glaring / obvious that needs addressing. In terms of the failing tests... Has there been any significant changes to the resolver recently? It seems to me that the await Migrate.run(config) step is failing in the tests - and it's not obvious the best way to fix it, but I'll be able to dig a little deeper into it tomorrow if it's not obvious to you what changed.

eggplantzzz · 2020-10-06T14:57:27Z

Sorry about jerking you back and forth on the package-identifier stuff, I didn't see @gnidan's suggestion about it until afterwards haha.

eggplantzzz · 2020-10-06T15:16:18Z

As for the CI failures, I don't think anything changed with the migrations. I'll take a quick look and see if I can find anything out about this.

eggplantzzz · 2020-10-06T16:10:31Z

So @njgheorghita, I took a look and your tests are failing because there were a bunch of breaking changes to some of the compilation packages. workflow-compile had compilation and the saving of artifacts separated into two steps. I made the necessary corrections and pushed them to a branch in the Truffle repo called fix-ethpm-tests. I didn't want to push directly to your branch but feel free to merge it into yours or fix it yourself. The gist of the matter is that Contracts.compile no longer saves the artifacts. There is now a method Contracts.compileAndSave that you need to use to do both. We also started using WorkflowCompile as the name when importing that library and I made that change in there as well. Let me know if something is unclear or if you need anything else!

njgheorghita · 2020-10-07T09:12:59Z

@eggplantzzz Nailed it! Thanks for looking into it and the fix!

packages/ethpm-v3/lib/utils.js

packages/ethpm-v1/lib/ethpm-v1.js

packages/resolver/lib/sources/ethpm-v3.ts

njgheorghita · 2020-10-27T16:03:01Z

@eggplantzzz I can definitely update packageIdentifier -> package_identifier. Just to make sure I'm not messing up anything here - what's the reason to merge the develop branch into this one? Is it fine to continue building on top of it?

njgheorghita · 2020-10-27T16:49:38Z

I've also updated the ethpm x truffle docs here and added my blog (which is fairly similar to the docs but has a better example and more in-depth analysis) here.

eggplantzzz · 2020-10-28T17:28:39Z

@njgheorghita Oh, I think there was just a merge conflict so I resolved it for you. I hope you don't mind...sorry if I stepped on your toes. It should be fine to continue as normal. And thanks for the fix!

njgheorghita mentioned this pull request Aug 10, 2020

Support ethpm V2 #2857

Closed

njgheorghita force-pushed the ethpm-v3 branch 12 times, most recently from 34ebb36 to 46618af Compare August 11, 2020 00:44

njgheorghita commented Aug 11, 2020

View reviewed changes

gnidan reviewed Aug 11, 2020

View reviewed changes

njgheorghita force-pushed the ethpm-v3 branch 6 times, most recently from bc0f2df to 573f9a0 Compare August 13, 2020 02:07

DiscRisknBisque mentioned this pull request Aug 14, 2020

Update ethpm docs to reflect v3 changes ConsenSys-archive/trufflesuite.com#620

Closed

njgheorghita force-pushed the ethpm-v3 branch 7 times, most recently from 9c496e7 to 2d2a688 Compare August 18, 2020 03:13

njgheorghita mentioned this pull request Aug 18, 2020

Replace module-alias with ttypescript ethpm/ethpm.js#47

Merged

eggplantzzz reviewed Oct 5, 2020

View reviewed changes

packages/ethpm-v3/lib/utils.js Outdated Show resolved Hide resolved

eggplantzzz reviewed Oct 5, 2020

View reviewed changes

packages/ethpm-v3/lib/utils.js Outdated Show resolved Hide resolved

eggplantzzz reviewed Oct 5, 2020

View reviewed changes

packages/ethpm-v3/lib/utils.js Outdated Show resolved Hide resolved

eggplantzzz reviewed Oct 5, 2020

View reviewed changes

packages/ethpm-v3/lib/utils.js Outdated Show resolved Hide resolved

njgheorghita force-pushed the ethpm-v3 branch from eab09a8 to 682ddc0 Compare October 6, 2020 14:07

njgheorghita force-pushed the ethpm-v3 branch 2 times, most recently from d3a25a0 to 32736ce Compare October 7, 2020 08:38

eggplantzzz reviewed Oct 12, 2020

View reviewed changes

njgheorghita force-pushed the ethpm-v3 branch 2 times, most recently from 8578562 to 00a55ef Compare October 19, 2020 10:03

njgheorghita added 7 commits October 30, 2020 13:15

Support v1 & v3

f5edfe5

Fix v3 tests

cd73d33

Update readme

164030b

Support listing packages for ethpm v1

4780edb

Fix failing tests

4b1b100

PR feedback

de6f7a2

More pr feedback

3ad36aa

njgheorghita force-pushed the ethpm-v3 branch from e4f0d73 to 3ad36aa Compare October 30, 2020 12:16

njgheorghita and others added 2 commits October 30, 2020 13:19

Update packageIdentifier arg to snake_case

2843ead

Merge branch 'develop' into ethpm-v3

0a5de23

eggplantzzz merged commit 3cc229d into ConsenSys-archive:develop Nov 3, 2020

gnidan mentioned this pull request Nov 4, 2020

Revert "ethpmV3 support" #3502

Merged

	usage: "truffle install <packageId> [--alias]",
	usage: "truffle install <package-identifier> [--alias]",

Conversation

njgheorghita commented Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

njgheorghita Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

gnidan Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

njgheorghita Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

njgheorghita Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gnidan left a comment

Choose a reason for hiding this comment

Uh oh!

gnidan Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

gnidan Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

njgheorghita commented Oct 6, 2020

Uh oh!

eggplantzzz commented Oct 6, 2020

Uh oh!

eggplantzzz commented Oct 6, 2020

Uh oh!

eggplantzzz commented Oct 6, 2020

Uh oh!

njgheorghita commented Oct 7, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

njgheorghita commented Oct 27, 2020

Uh oh!

njgheorghita commented Oct 27, 2020

Uh oh!

eggplantzzz commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

njgheorghita commented Aug 10, 2020 •

edited

Loading

njgheorghita Aug 12, 2020 •

edited

Loading

eggplantzzz commented Oct 28, 2020 •

edited

Loading