Update `truffle init` artifacts #5320

sukanyaparashar · 2022-07-18T19:24:33Z

ISSUE

Update truffle init artifacts. See #5268.

SOLUTION

This PR follows the below mentioned steps to update truffle init artifacts -

Remove Migrations.sol from contracts.
Remove 1_initial_migration.js from migrations.
Update the sample truffle-config.js including the following -
- Use Goerli or Sepolia as the testnet.
- Change .secret to .env.
- Get mnemonic and Infura project id from .env.
- The comment says auto create at 9545, but the development environment config is 8545. What should be the ideal behaviour? I guess, the comment needs to be changed and use managed Ganache instance instead of development blockchain.
Add a .gitignore with .env in it.

cliffoo

Thanks @sukanyaparashar for taking this on! I've left a few comments. Note that issue#5268 also mentioned changing the comment on line 35 in truffle-config.js (from port 9545 to 8545)

cliffoo · 2022-07-23T04:17:23Z

packages/core/lib/commands/init/initSource/.gitignore

@@ -0,0 +1 @@
+.env


Let's have a newline at the end.

cliffoo · 2022-07-23T04:21:30Z

packages/core/lib/commands/init/initSource/truffle-config.js

+ * If you want to use .env for keeping your secret variables, you'll need to install 'dotenv'
+ * (npm install dotenv). To get started with, add .env file in the root directory and declare
+ * your Infura MNEMONIC and PROJECT_ID variables.
+ * .gitignore is already added with this project with .env in it.
 */

+// require('dotenv').config();
 // const HDWalletProvider = require('@truffle/hdwallet-provider');
-//
-// const fs = require('fs');
-// const mnemonic = fs.readFileSync(".secret").toString().trim();
+
+// const mnemonic = process.env.MNEMONIC;
+// const projectId = process.env.PROJECT_ID;


Since the default we give users is reading from process.env, that means we want users to use dotenv right? So I would phrase this as:

It is recommended to store your secret variables in a .env file (which you should include in .gitignore). In your project root, run npm i dotenv , create .env and declare your MNEMONIC and infura PROJECT_ID variables inside.

Also destructure maybe? const { MNEMONIC, PROJECT_ID } = process.env;

Your suggestions make sense to me. I will change the comments. Thanks @cliffoo.

cliffoo · 2022-07-23T04:22:49Z

packages/core/lib/commands/init/initSource/truffle-config.js

+    // goerli: {
+    //   provider: () => new HDWalletProvider(mnemonic, `https://goerli.infura.io/v3/${projectId}`),
+    //   network_id: 5,       // Goerli's id
+    //   gas: 5500000,        // Goerli has a lower block limit than mainnet


Does the comment about gas hold true for Goerli?

I'm not sure if it does and maybe this is not very helpful in here. What do you think? Should we just get rid of this gas line altogether?

I couldn't find any suitable documentation for this gas line. I guess we can just get rid of this for now.

@gnidan @haltman-at Do you guys have any insight on where this gas comment is from? Should it be kept / changed now that it's goerli?

packages/core/lib/commands/init/initSource/truffle-config.js

cliffoo · 2022-07-23T04:23:32Z

packages/core/lib/commands/init/initSource/truffle-config.js

@@ -94,7 +99,7 @@ module.exports = {
      //  evmVersion: "byzantium"
      // }
    }
-  },
+  }


This comma should stay.

prettier again :(

Same, please don't use --no-verify flag on commits.

So what should you do when committing non-JS files or in situations like this where you don't want prettier to update things? I think in this case using that flag should be fine. Do you disagree or have a different solution?

We were discussing that if we can somehow exclude some files from using prettier. I have raised an issue for this #5360. For now, lets use --no-verify for this PR. And find a way to resolve this prettier issue later.

sukanyaparashar · 2022-07-26T22:52:59Z

Thanks @sukanyaparashar for taking this on! I've left a few comments. Note that issue#5268 also mentioned changing the comment on line 35 in truffle-config.js (from port 9545 to 8545)

Thanks for the review @cliffoo. In regards to the point of changing the port from 9545 to 8545, there is a catch here. Port 9545 is used by the managed Ganache when truffle develop is run with no network config.

On the other hand, when truffle test is run, it looks for the development network settings in the config by default, where 8545 is mentioned (Note: ganache-cli has the default port 8545).

Any thoughts on what should be the ideal default values for the development configuration?

cliffoo · 2022-08-04T18:40:01Z

@sukanyaparashar Yea I think the development network config is fine, the confusion comes from here, where it mentions a "development blockchain", which makes one wonder if it's referring to the development network (it isn't). I don't really know how to call it instead, "dev"? "ephemeral"? Your call 😅

sukanyaparashar · 2022-08-04T18:51:49Z

@sukanyaparashar Yea I think the development network config is fine, the confusion comes from here, where it mentions a "development blockchain", which makes one wonder if it's referring to the development network (it isn't). I don't really know how to call it instead, "dev"? "ephemeral"? Your call 😅

I guess a better name will be "managed Ganache instance" instead of a "development blockchain", may be?

cliffoo · 2022-08-04T22:32:44Z

Sounds fine by me 👍 It's nice that it maps to the variable name in code.

eggplantzzz

cooool - hit approval but if there is anything left to address from the comments above then do that first before merging :)

sukanyaparashar · 2022-08-05T19:13:40Z

cooool - hit approval but if there is anything left to address from the comments above then do that first before merging :)

Thanks for your approval @eggplantzzz. Yeah! I need to fix few comments and wait for @cliffoo's approval as well.

…comments

cliffoo

Thanks @sukanyaparashar! This is great. I'm just being picky about a few things, feel free to (not) make changes as you see fit. LGTM!

cliffoo · 2022-08-06T08:52:59Z

packages/core/lib/commands/init/initSource/truffle-config.js


 module.exports = {
  /**
   * Networks define how you connect to your ethereum client and let you set the
   * defaults web3 uses to send transactions. If you don't specify one truffle
-   * will spin up a development blockchain for you on port 9545 when you
+   * will spin up a managed Ganache instance for you on port 9545 when you


cliffoo · 2022-08-06T08:53:04Z

packages/core/lib/commands/init/initSource/truffle-config.js

    solc: {
-      version: "0.8.15",      // Fetch exact version from solc-bin (default: truffle's version)
+      version: "0.8.15", // Fetch exact version from solc-bin (default: truffle's version)


It looks like this from here isn't actually resolved.

The whitespace.

Yeah! I guess I missed that. Will do the required changes. Thanks @cliffoo.

cliffoo · 2022-08-06T08:55:54Z

packages/core/lib/commands/init/initSource/truffle-config.js

+// require('dotenv').config();
 // const HDWalletProvider = require('@truffle/hdwallet-provider');
-//
-// const fs = require('fs');
-// const mnemonic = fs.readFileSync(".secret").toString().trim();
+
+// const { MNEMONIC, PROJECT_ID } = process.env;


Maybe the require dotenv line should be adjacent to the destructure line, and separated from the hdwallet-provider line? Like so:

require dotenv destructure hdwallet-provider

Yes, we can do this to make dotenv usage clearer. I will change this. Thanks for pointing that out @cliffoo.

cliffoo

Nice one! Thanks @sukanyaparashar

sukanyaparashar · 2022-08-09T01:03:39Z

Nice one! Thanks @sukanyaparashar

Thanks for the approval @cliffoo. I am making few more changes to the truffle-config.js file comments, after discussing with @cds-amal and @gnidan to make it more appealing to the users to read (like recommending to use truffle dashboard for better security practices).

cliffoo

Nice one @sukanyaparashar! Definitely a plus to mention dashboard and various trade-offs. My comments are merely suggestions, please proceed with whatever you think is clear and informative 👍

cliffoo · 2022-08-11T09:43:32Z

packages/core/lib/commands/init/initSource/truffle-config.js

+ * PLEASE NOTE 🗒️: This is an insecure workflow! It is possible through human error to leak 
+ * your mnemonic on Github. 


I wouldn't sell hdwallet-provider as insecure haha. Maybe it's okay to end it off here? Since it's mentioned above that you shouldn't commit .env

I, kind of agree with you @cliffoo. We may not want to directly say hdwallet-provider as "insecure". We have already mentioned to add .env in .gitignore.

cliffoo · 2022-08-11T10:00:00Z

packages/core/lib/commands/init/initSource/truffle-config.js

+ * Are you concerned about the security 🤔? Use this approach to get rid of this hassle 🤦‍♀️:
+ * 
+ * A more secure 🔒 workflow is to use truffle dashboard which leverages
+ * Metamask for signing your transactions and does not require you to reveal your mnemonic to Truffle.


The first sentence isn't so clear about what the security and this hassle are. Perhaps this section can be more succinct? Something like:

Are you concerned about security? Truffle dashboard lets you review transactions in detail, and leverages MetaMask for signing so there's no need to copy-paste your mnemonic.

Of course you can insert emojis to your heart's content 🦄

cliffoo · 2022-08-11T10:00:05Z

packages/core/lib/commands/init/initSource/truffle-config.js

@@ -73,7 +87,7 @@ module.exports = {
    //
    // Useful for private networks
    // private: {
-    //   provider: () => new HDWalletProvider(mnemonic, `https://network.io`),
+    //   provider: () => new HDWalletProvider(MNEMONIC, `https://network.io`),


Nice catch!

cliffoo · 2022-08-11T18:54:59Z

packages/core/lib/commands/init/initSource/truffle-config.js

- * 
- * A more secure 🔒 workflow is to use truffle dashboard which leverages
- * Metamask for signing your transactions and does not require you to reveal your mnemonic to Truffle.
+ * Are you concerned about the security 🤔? Truffle dashboard lets you review transactions in detail, 


Final pick, can you remove the in this? All good on my side after this ;)

Oh Okay! Cool! :)

gnidan

The new truffle-config.js writeup looks great! Just a couple notes

gnidan · 2022-08-11T19:13:10Z

packages/core/lib/commands/init/initSource/truffle-config.js

+ * Deployment with Truffle Dashboard
+ * ---------------------------------
+ * 
+ * Are you concerned about the security 🤔? Truffle dashboard lets you review transactions in detail, 


Suggested change

* Are you concerned about the security 🤔? Truffle dashboard lets you review transactions in detail,

* Are you concerned about the security 🤔? Truffle Dashboard lets you review transactions in detail,

gnidan · 2022-08-11T19:17:04Z

packages/core/lib/commands/init/initSource/truffle-config.js

+ * 
+ * Are you concerned about the security 🤔? Truffle dashboard lets you review transactions in detail, 
+ * and leverages MetaMask for signing, so there's no need to copy-paste your mnemonic.
+ * Please see this link for more details 🔎: https://trufflesuite.com/docs/truffle/getting-started/using-the-truffle-dashboard/


Could you please make sure that all comments fit under 80 characters per line? (My comment here applies generally; this line is just the one that stood out as longest)

sukanyaparashar · 2022-08-11T19:24:51Z

The new truffle-config.js writeup looks great! Just a couple notes

Thanks for the quick review, @gnidan. I will make the suggested changes. :)

cds-amal

Thanks for raising security awareness here @sukanyaparashar! Looks good to me.

gnidan · 2022-08-12T04:06:54Z

packages/core/lib/commands/init/initSource/truffle-config.js

@@ -11,25 +11,31 @@
 * Hands-off deployment with Infura
 * --------------------------------
 *
- * Is your application complex and requires lots of transactions to deploy?
+ * Is your application complex and require lots of transactions to deploy?


Y'know, figuring out whether this should be singular or plural would be a lot easier if this question was correct grammar in the first place... proper grammar here would be "Is your application complex and does it require..." but that sounds so wordy...

Maybe this should be "Do you have a complex application that requires lots of transactions to deploy?" or something that doesn't require combining clauses with an "and".

Sorry for the poor grammar in the initial wording I wrote for this!

Do you have a complex application that requires lots of transactions to deploy?

I like it

No worries! @gnidan. I agree with @cds-amal. I like this "Do you have a complex application that requires lots of transactions to deploy?" wording too. Simple grammar ;)

Thanks for making those edits!

gnidan

have another approve :)

cliffoo

Have yet another one

sukanyaparashar marked this pull request as draft July 18, 2022 19:24

sukanyaparashar marked this pull request as ready for review July 22, 2022 17:16

sukanyaparashar requested review from cds-amal, eggplantzzz, cliffoo, emilyJLin95, gnidan, lsqproduction and haltman-at July 22, 2022 17:16

cliffoo suggested changes Jul 23, 2022

View reviewed changes

sukanyaparashar requested a review from cliffoo August 3, 2022 03:31

eggplantzzz approved these changes Aug 5, 2022

View reviewed changes

sukanyaparashar added 8 commits August 5, 2022 13:17

Remove Migrations.sol and 1_initial_migration.js

b03435e

Change public testnet to Goerli

3e9157a

Add .env and .gitignore

3968c4a

Change comments for .env and .gitignore

25e8f62

Change comments for .env and .gitignore

3ac0ae7

Fix comments for .env variables

407bd3e

Add trailing commas

af224e7

Use managed Ganache instance replacing development blockchain in the …

c4bd079

…comments

sukanyaparashar force-pushed the update-truffle-init branch from 1dce489 to c4bd079 Compare August 5, 2022 19:23

cliffoo approved these changes Aug 6, 2022

View reviewed changes

Add the required whitespaces in the comments

f74ebba

sukanyaparashar requested review from eggplantzzz and cliffoo August 8, 2022 18:58

cliffoo approved these changes Aug 8, 2022

View reviewed changes

Add pretty icons in the comments

2db0680

sukanyaparashar requested a review from cliffoo August 10, 2022 21:34

cliffoo approved these changes Aug 11, 2022

View reviewed changes

Some more changes in the comments

8543346

sukanyaparashar requested a review from cliffoo August 11, 2022 18:44

cliffoo approved these changes Aug 11, 2022

View reviewed changes

gnidan previously requested changes Aug 11, 2022

View reviewed changes

Limit the comments to 80 characters per line

430b4d6

sukanyaparashar requested review from gnidan and cliffoo August 11, 2022 19:40

sukanyaparashar added 2 commits August 11, 2022 15:53

Minor edits and conform text to 80 columns

6590937

Add new line

baa425c

cds-amal approved these changes Aug 12, 2022

View reviewed changes

gnidan reviewed Aug 12, 2022

View reviewed changes

Some grammar improvement

1a5221f

gnidan approved these changes Aug 12, 2022

View reviewed changes

cliffoo approved these changes Aug 12, 2022

View reviewed changes

sukanyaparashar merged commit 9560b32 into develop Aug 12, 2022

sukanyaparashar deleted the update-truffle-init branch August 12, 2022 18:39

kevinbluer mentioned this pull request Aug 18, 2022

Update truffle init artifacts #5268

Open

8 tasks

		* PLEASE NOTE 🗒️: This is an insecure workflow! It is possible through human error to leak
		* your mnemonic on Github.

	* Are you concerned about the security 🤔? Truffle dashboard lets you review transactions in detail,
	* Are you concerned about the security 🤔? Truffle Dashboard lets you review transactions in detail,

Update truffle init artifacts #5320

Update truffle init artifacts #5320

Conversation

sukanyaparashar commented Jul 18, 2022 • edited Loading

ISSUE

SOLUTION

cliffoo left a 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

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

sukanyaparashar Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

sukanyaparashar commented Jul 26, 2022 • edited Loading

cliffoo commented Aug 4, 2022

sukanyaparashar commented Aug 4, 2022

cliffoo commented Aug 4, 2022

eggplantzzz left a comment • edited Loading

Choose a reason for hiding this comment

sukanyaparashar commented Aug 5, 2022

cliffoo left a 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

cliffoo left a comment

Choose a reason for hiding this comment

sukanyaparashar commented Aug 9, 2022

cliffoo left a comment

Choose a reason for hiding this comment

cliffoo Aug 11, 2022 • 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

gnidan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sukanyaparashar commented Aug 11, 2022

cds-amal left a 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

gnidan left a comment

Choose a reason for hiding this comment

cliffoo left a comment

Choose a reason for hiding this comment

Update `truffle init` artifacts #5320

Update `truffle init` artifacts #5320

sukanyaparashar commented Jul 18, 2022 •

edited

Loading

sukanyaparashar Aug 3, 2022 •

edited

Loading

sukanyaparashar commented Jul 26, 2022 •

edited

Loading

eggplantzzz left a comment •

edited

Loading

cliffoo Aug 11, 2022 •

edited

Loading