Skip to content

Conversation

@amateima
Copy link
Contributor

@amateima amateima commented Sep 5, 2022

This PRs adds the contract source code and tests for the Merkle Distributor contract. It is based on the already audited contract from the UMA protocol repository

Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

Looks great! Just two high level questions -- trying to understand how much of this is copied code vs freshly written.

@@ -0,0 +1,257 @@
// SPDX-License-Identifier: AGPL-3.0-only
Copy link
Contributor

Choose a reason for hiding this comment

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

When you say "based on" do you mean directly copied? Or are there changes? If there are changes, we will likely need to have this re-audited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The contract itself is identical

@@ -0,0 +1,692 @@
import { assert } from "chai";
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here: is this copied code or should we review it? If copied, were there any specific changes we should look at?

Copy link
Contributor Author

@amateima amateima Sep 7, 2022

Choose a reason for hiding this comment

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

The tests structure is the same, only minor things were changed: replace web3 usage with ethers and update the assertion syntax (use expect instead of assert in order to have a similar syntax with the rest of the tests)

@amateima amateima force-pushed the amatei/acx-148-move-merkledistributor-contracts-scripts branch from 9b39990 to a29e0e1 Compare September 7, 2022 11:16
Signed-off-by: amatei <amatei@umaproject.org>
Signed-off-by: amatei <amatei@umaproject.org>
@amateima amateima force-pushed the amatei/acx-148-move-merkledistributor-contracts-scripts branch from a29e0e1 to 6a68f0c Compare September 7, 2022 11:26
@socket-security
Copy link

Socket Security Report

Dependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again.

📜 New install scripts detected

A dependency change in this PR is introducing new install scripts to your install step.

Package Script field Location
ursa-optional@0.10.2 (added) binding.gyp package.json via @uma/merkle-distributor@1.3.27, ipfs-http-client@49.0.4, ipfs-core-types@0.3.1, peer-id@0.14.8, libp2p-crypto@0.19.7
protobufjs@6.11.3 (added) postinstall package.json via @uma/common@2.20.0, @google-cloud/kms@2.11.0, google-gax@2.30.0, @grpc/proto-loader@0.6.9
ursa-optional@0.10.2 (added) install package.json via @uma/merkle-distributor@1.3.27, ipfs-http-client@49.0.4, ipfs-core-types@0.3.1, peer-id@0.14.8, libp2p-crypto@0.19.7
iso-constants@0.1.2 (added) install package.json via @uma/merkle-distributor@1.3.27, ipfs-http-client@49.0.4, it-tar@1.2.2
🫣 Native code

Contains native code which could be a vector to obscure malicious code, and generally decrease the likelihood of reproducible or reliable installs.

Package Location
ursa-optional@0.10.2 (added) package.json via @uma/merkle-distributor@1.3.27, ipfs-http-client@49.0.4, ipfs-core-types@0.3.1, peer-id@0.14.8, libp2p-crypto@0.19.7
Socket.dev scan summary
Issue Status
Did you mean? ✅ no new possible package typos
Install scripts ⚠️ 4 new install scripts detected
Telemetry ✅ no new telemetry
Troll package ✅ no new troll packages
Malware ✅ no new malware
Native code ⚠️ 1 new native module detected

Powered by socket.dev

@nicholaspai nicholaspai merged commit f42e843 into master Sep 17, 2022
@nicholaspai nicholaspai deleted the amatei/acx-148-move-merkledistributor-contracts-scripts branch September 17, 2022 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants