-
Notifications
You must be signed in to change notification settings - Fork 368
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
checkDiff validates Etherscan source code #192
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just left some minor comments. Another feature that may be nice to add in the future would be checking the ABI generated by truffle against the ABI on etherscan.
validate/contractDiff.js
Outdated
} | ||
} | ||
|
||
console.log(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few empty console.log statements in this block that can be deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced console.log()
with extra "\n" in process.stdout.write(...)
statements.
validate/contractDiff.js
Outdated
console.log() | ||
process.stdout.write(chalk.green("Successfully processed " + (total-fail) + " files.\n")); | ||
process.stdout.write(chalk.green(goodFiles)); | ||
process.stdout.write(chalk.red("\n\nFailed to process " + fail + " files.\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we only print this if fail > 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
validate/README.checkDiff.md
Outdated
The `checkDiff` script compares source code uploaded to Etherscan to source | ||
code on the local machine. | ||
|
||
The souce code in Etherscan is a concatenation of several files. The script reads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: souce -> source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
validate/README.checkDiff.md
Outdated
|
||
1. Comments / whitespace | ||
2. The local code has `import` statements not included in Etherscan because the files were | ||
explictly concatenated into a master file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: explictly -> explicitly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Created CENT-257 to create ABI validation script in USDC v2. |
…irclefin#192) ## Summary This pull request modifies `FiatTokenCeloV2_2` to allow greater flexibility in who the fee caller for `debitGasFees` and `creditGasFees` can be. Circle has decided to pursue an approach (pending Security, Rui Maximo) in which yet another contract in the call stack calls `debitGasFees` and `creditGasFees`. To that end, we cannot rely on our assumption of 0x0 being the fee caller anymore. ## Detail - Modified FTCV2.2 to account for a dynamic fee caller. This is stored in a Keccak256 storage slot to prevent modification to the USDC storage layout. - Ensured that only the token owner can call `updateFeeCaller`, similar to existing procedures for the blacklister, pauser, etc. - Modified contracts where appropriate and added tests. ## Testing See the tests. ## Documentation - [`FiatTokenFeeAdapter` rollout plan/timeline](https://docs.google.com/document/d/19qISUWAft_yCzg5lPrf41f2m3Bxgi7RQTIqeLZ6_Ghw/edit) - [Product document](https://docs.google.com/document/d/1YKVpFo9aPuBREX7eXcXinFLpskF5q9nUxvbBd0ifYl8) ## Sanitization - [x] Will this commit be publicized? Depending on whether we squash during open-sourcing, it may or may not be. @zhenghui-w is handling the open-sourcing effort. For PR author, have you: - [x] Checked that PR title does not contain internal references? (eg. internal code names, employee names) - [x] Checked that commit messages do not contain internal references? (eg. internal code names, employee names) - [x] Checked that all new code contain the Apache 2.0 copyright license header - [x] Checked that all new third party dependencies use permissive licenses For reviewers, ensure that: - The PR is squash merged - JIRA story IDs are removed from the squash merge commit title Refer to the [Open Source Standard](https://drive.google.com/file/d/1OV5E9ZOBmyZeN877vPP02C92Mjm-qt4Z/view) for more information about sanitization. ## Story STABLE-5628
Created a script that compares the source code downloaded from Etherscan to files stored on the local machine. It reads the Etherscan code to learn which local files to use, then reads the local files and concatenates them into an "expected" source file. Then it shows a diff. Usage instructions are in validate/README.checkDiff.md.
Also included copies of files downloaded directly from Etherscan: FiatTokenProxy.etherscan and FiatTokenV1.etherscan. These are for testing purposes only. Actual validation should be performed by manually downloading the files from Etherscan (instructions in validate/README.checkDiff.md).