Featuer/HOLO-605: C4 medium risk fixes #88

ACC01ADE · 2022-11-17T00:04:20Z

Describe Changes

I made this more better by doing ...

Merged the HolographGeneric implementation in here
Fixed security signing checks
Switched to per-network gas pricing and calculations

Related Code4rena issues fixed

Checklist before requesting a review

I have performed a self-review of my code
Code styles have been enforced
All Hardhat tests are passing

* fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix

alexanderattar · 2022-11-23T16:16:46Z

src/HolographFactory.sol

@@ -228,6 +228,15 @@ contract HolographFactory is Admin, Initializable, Holographable, HolographFacto
    if (v < 27) {
      v += 27;
    }
+    if (v != 27 && v != 28) {


Could you add a note about this logic and hashes.

I found some documentation that looks related in the OZ repo, but this is a bit different:

// EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature // unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines // the valid range for s in (301): 0 < s < secp256k1n ÷ 2 + 1, and for v in (302): v ∈ {27, 28}. Most // signatures from current libraries generate a unique signature with an s-value in the lower half order. // // If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept // these malleable signatures as well.

Notes and details added in latest commit. Also updated and documented the functionality in enforcer/HolographERC20.sol.

alexanderattar · 2022-11-23T16:20:53Z

src/HolographOperator.sol

@@ -836,7 +841,7 @@ contract HolographOperator is Admin, Initializable, HolographOperatorInterface {
      /**
       * @dev check if smart contract is owned by sender
       */
-      require(Ownable(operator).isOwner(msg.sender), "HOLOGRAPH: sender not owner");
+      require(Ownable(operator).owner() == msg.sender, "HOLOGRAPH: sender not owner");


Just curious what the difference is between these two functions is for future reference

For ownable type, owner function is default. IsOwner is not in spec and will not always be present. So we went more strict and are using a function that is by the spec.

ah okay cool

alexanderattar · 2022-11-23T16:29:06Z

src/abstract/GenericH.sol

+
+import "../abstract/Initializable.sol";
+
+abstract contract GenericH is Initializable {


What is this contract for? A example contract implementation contract?

This is an abstract contract that any generic implementations would need to use and have a minimum of functions supported in order to be able to interact with our protocol. So just like we currently have ERC20 and ERC72, this one is the same but without being tied to a specific spec.

got it. cool!

alexanderattar · 2022-11-23T16:34:43Z

src/enforcer/HolographERC20.sol

+            data
+          )
+        )
+      );


Should this include an error message if reverts?

The _sourceCall function already captures and handles the reverts in assembly.

alexanderattar · 2022-11-23T16:35:04Z

src/enforcer/HolographERC20.sol

+            data
+          )
+        )
+      );


Same here and for other instances of _sourceCall where wrapped in require?

The _sourceCall function already captures and handles the reverts in assembly.

alexanderattar

Reviewed and ran tests successfully 👍

alexanderattar · 2022-11-23T16:56:58Z

test/03_erc20_enforcer_tests_l1.ts

@@ -368,19 +368,19 @@ describe('Testing the Holograph ERC20 Enforcer (L1)', async function () {
          .withArgs(l1.deployer.address, l1.wallet2.address, 0);
      });

-      it('should fail for non-contract onERC20Received call', async function () {
+      it.skip('should fail for non-contract onERC20Received call', async function () {


Are these to be reenabled later

We removed the erc20 receiver since it’s not a standard or widely used spec. These will be skipped for now and maybe removed entirely if we do not implement an alternative safe transfer function.

sogoiii · 2022-11-23T17:54:38Z

src/enforcer/HolographERC721.sol

@@ -321,7 +324,30 @@ contract HolographERC721 is Admin, Owner, HolographERC721Interface, Initializabl
    require(_isApproved(sender, tokenId), "ERC721: sender not approved");
    require(from == _tokenOwner[tokenId], "ERC721: from is not owner");
    if (_isEventRegistered(HolographERC721Event.bridgeOut)) {
-      data = SourceERC721().bridgeOut(toChain, from, to, tokenId);
+      bytes memory sourcePayload = abi.encodeWithSelector(


@ACC01ADE can we make a comment about this? It looks like this is going to call the bridgeOut function but its not obvious.

Added commentary to this on ERC721 and ERC20. Also provided commentary on _sourceCall.

sogoiii · 2022-11-23T17:58:49Z

src/enforcer/HolographERC721.sol

@@ -901,6 +949,21 @@ contract HolographERC721 is Admin, Owner, HolographERC721Interface, Initializabl
    }
  }

+  function _sourceCall(bytes memory payload) private returns (bool output) {


What is this function protecting?

Since we use msgSender function in place of msg.sender for source contract interactions between holographer, there are cases where we made calls without that being enabled/present. This creates an issue where custom contracts can make the assumption that the msgSender will always return data which was not the case. This change in code fixes the issue.

sogoiii · 2022-11-23T17:59:42Z

src/token/HolographUtilityToken.sol

+  /**
+   * @dev Temporarily placed to bypass bytecode conflicts
+   */
+  function isHLG() external pure returns (bool) {


When will we remove this given the bycode conflict?

This was temporarily added so that I could run extensive onchain tests on top of the regular localhost tests. Will be removed when we are ready to change deployment salt.

sogoiii · 2022-11-23T17:59:51Z

src/token/hToken.sol

+  /**
+   * @dev Temporarily placed to bypass bytecode conflicts
+   */
+  function isHToken() external pure returns (bool) {


When will we remove this given the bycode conflict?

This was temporarily added so that I could run extensive onchain tests on top of the regular localhost tests. Will be removed when we are ready to change deployment salt.

sogoiii

👍

* Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io>

* Merge experimental * Experimental to Develop 20221206 (#95) * Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io>

* Release/develop to testnet 20221207 (#96) * Merge experimental * Experimental to Develop 20221206 (#95) * Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest testnet deployments and abis (#97) * Add latest testnet deployments and abis * Update deployment salt history file for clarity * local changes * updates * fixes * clearer check for false * fixing nonce issue * multisig transfer * mainnet upgrade test * switching to networks npm package for multisig reference * cleanup on aisle 9 * fixed nonce bug for tests and adding recoverJob tests * nonce fix * test gas limit adjustment * adding details to bad gas for test Co-authored-by: Alexander <alexanderattar@users.noreply.github.com> Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com>

* Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 * Merge develop * HOLO-678: Deployment patches (#98) * Release/develop to testnet 20221207 (#96) * Merge experimental * Experimental to Develop 20221206 (#95) * Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest testnet deployments and abis (#97) * Add latest testnet deployments and abis * Update deployment salt history file for clarity * local changes * updates * fixes * clearer check for false * fixing nonce issue * multisig transfer * mainnet upgrade test * switching to networks npm package for multisig reference * cleanup on aisle 9 * fixed nonce bug for tests and adding recoverJob tests * nonce fix * test gas limit adjustment * adding details to bad gas for test Co-authored-by: Alexander <alexanderattar@users.noreply.github.com> Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> * Add code4rena audit report (#100) Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io>

* Merge experimental * Experimental to Develop 20221206 (#95) * Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 * Merge testnet to develop 20221208 * Experimental to develop (#101) * Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 * Merge develop * HOLO-678: Deployment patches (#98) * Release/develop to testnet 20221207 (#96) * Merge experimental * Experimental to Develop 20221206 (#95) * Improvement/ HOLO 595 Enforce prettier / lint run on protocol (#81) * prettier and eslint setup to run action * fixed command on prettier action * fixed command on prettier action and package.json * fixed command on prettier action * Remove unknown prettier options * husky prepush check for linting and prettier * added husky prepare on package.json * fixed husky prepare on package.json * fixed husky pre-push Co-authored-by: Alexander <alexanderattar@gmail.com> * Add solhint config and fix prettier config (#83) * Feature/HOLO-604: implementing critical issue fixes (#84) * fixing critical issues * implemented suggestions * Featuer/HOLO-605: C4 medium risk fixes (#88) * init * fixes * enforcing msgSender on all source contract calls * fixing typo * fixing tests * test fixes and prettier * royatlies patch * removing unused library * Feature/adding generic contract type (#85) * fixing critical issues * adding generic contract type * implemented suggestions * merging latest from experimental branch * adding withdraw andmsgSender protection * adding withdraw andmsgSender protection * prettier * fixing typo * assembly memory fix * combined generic contract pr * deployments * deployments * adding support for `asciihex` compiler function * adding comments and fixing missed check * Feature/holo 613 rename pa1d to royalty (#90) * name change * Quick minor updates * Update reverts to use new ROYALTIES format * fix to test Co-authored-by: Vitto <admin@vitto.io> * royalties hotfix (#91) * royalties change * develop env deployments of royalties hotfix * Feature/holo 612 royalty smart contract improvements (#93) * First pass at royalty contract improvements * Second pass on royalty improvements from C4 audit * Remove broken check * Minor check and comments added * Remove check for greater than 10000 tokens for ERC20s in royalties * Add usage notes for payout functions * Add logic to allow setting a slot to use either transfer or call * Add handling for code-423n4/2022-10-holograph-findings#456 * Fix tests by passing proper init code * Add dev note on _callOptionalReturn * Limit payout addresses to 10 * Add test for max addresses * Improvement/holo 614 royalties smart contracts tests (#86) * royalties distribution * removed comments Co-authored-by: Alexander <alexanderattar@gmail.com> * cleanup * check send amount on ethPayouts Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: Vitto <admin@vitto.io> * Feature/HOLO-642: Implement Super Cold Storage logic into protocol (#92) * clean * implementing the super-cold-storage-signer * cleanup * Latest deployments 20221206 * Add external deployments back * Roll back to ff5b4ee due to incorrect deployment process on experimental env * Add latest deployments 20221206 Wed Dec 7 03:08:37 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 * Add latest develop deployments Wed Dec 7 17:57:20 UTC 2022 Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> * Add latest testnet deployments and abis (#97) * Add latest testnet deployments and abis * Update deployment salt history file for clarity * local changes * updates * fixes * clearer check for false * fixing nonce issue * multisig transfer * mainnet upgrade test * switching to networks npm package for multisig reference * cleanup on aisle 9 * fixed nonce bug for tests and adding recoverJob tests * nonce fix * test gas limit adjustment * adding details to bad gas for test Co-authored-by: Alexander <alexanderattar@users.noreply.github.com> Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> * Add code4rena audit report (#100) Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io> Co-authored-by: Natalie Bravo <natalie.bravo@outlook.com> Co-authored-by: ACC01ADE <admin@vitto.io>

ACC01ADE added 11 commits November 17, 2022 01:03

init

fb6b450

fixes

09ad69d

enforcing msgSender on all source contract calls

33e8f00

fixing typo

0e4d3c5

fixing tests

184a386

test fixes and prettier

cd677c8

royatlies patch

2d0fd9b

removing unused library

fc6e556

combined generic contract pr

0f507ee

deployments

863d8be

ACC01ADE requested review from alexanderattar and sogoiii November 22, 2022 19:39

deployments

537c307

ACC01ADE marked this pull request as ready for review November 22, 2022 19:41

adding support for asciihex compiler function

5a83e4f

alexanderattar reviewed Nov 23, 2022

View reviewed changes

alexanderattar approved these changes Nov 23, 2022

View reviewed changes

alexanderattar reviewed Nov 23, 2022

View reviewed changes

sogoiii reviewed Nov 23, 2022

View reviewed changes

adding comments and fixing missed check

1db243d

alexanderattar approved these changes Nov 23, 2022

View reviewed changes

sogoiii approved these changes Nov 23, 2022

View reviewed changes

ACC01ADE merged commit c5193aa into experimental Nov 23, 2022

ACC01ADE deleted the feature/holo-605-c4-medium-risk-fixes branch November 23, 2022 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Featuer/HOLO-605: C4 medium risk fixes #88

Featuer/HOLO-605: C4 medium risk fixes #88

ACC01ADE commented Nov 17, 2022 •

edited

Loading

alexanderattar Nov 23, 2022 •

edited

Loading

ACC01ADE Nov 23, 2022

alexanderattar Nov 23, 2022

alexanderattar Nov 23, 2022

ACC01ADE Nov 23, 2022

alexanderattar Nov 23, 2022

alexanderattar Nov 23, 2022

ACC01ADE Nov 23, 2022

alexanderattar Nov 23, 2022

alexanderattar Nov 23, 2022

ACC01ADE Nov 23, 2022

alexanderattar Nov 23, 2022 •

edited

Loading

ACC01ADE Nov 23, 2022

alexanderattar left a comment

alexanderattar Nov 23, 2022

ACC01ADE Nov 23, 2022

sogoiii Nov 23, 2022

ACC01ADE Nov 23, 2022

sogoiii Nov 23, 2022

ACC01ADE Nov 23, 2022

sogoiii Nov 23, 2022

ACC01ADE Nov 23, 2022

sogoiii Nov 23, 2022

ACC01ADE Nov 23, 2022

sogoiii left a comment


		import "../abstract/Initializable.sol";

		abstract contract GenericH is Initializable {

Featuer/HOLO-605: C4 medium risk fixes #88

Featuer/HOLO-605: C4 medium risk fixes #88

Conversation

ACC01ADE commented Nov 17, 2022 • edited Loading

Describe Changes

Related Code4rena issues fixed

Checklist before requesting a review

alexanderattar Nov 23, 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

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

alexanderattar Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderattar 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

sogoiii left a comment

Choose a reason for hiding this comment

ACC01ADE commented Nov 17, 2022 •

edited

Loading

alexanderattar Nov 23, 2022 •

edited

Loading

alexanderattar Nov 23, 2022 •

edited

Loading