Check if the organization contracts are identical in all repository

[deepesh-kn]

Check if the following contracts are identical in all repository.

• Organization.sol
• Organized.sol
• OrganizationInterface.sol

If it is not identical then

• Find the differences.
• Discuss with the team.
• After discussion create the followup tickets

[jasonklein]

Findings

• Placement of organization contracts:
  • mosaic-contracts: /contracts/lib
  • brandedtoken-contracts: /contracts
  • openst-contracts: /contracts/organization
  • organization-contracts: /contracts
• Organization.sol
  • mosaic-contracts: import/using SafeMath.sol (no SafeMath functions used)
  • brandedtoken-contracts: n/a
  • openst-contracts: no import/using SafeMath.sol
  • organization-contracts: effectively identical to mosaic-contracts (SafeMath.sol is imported from /contracts/lib)
• Organized.sol
  • mosaic-contracts: copyright is 2019; 2 new lines after contract Organized { and not 1
  • brandedtoken-contracts: copyright is 2018; 1 new line after contract Organized { and not 2
  • openst-contracts: identical to mosaic-contracts
  • organization-contracts: identical to mosaic-contracts
• OrganizationInterface.sol:
  • mosaic-contracts: copyright is 2019
  • brandedtoken-contracts: copyright is 2018
  • openst-contracts: identical to mosaic-contracts
  • organization-contracts: identical to mosaic-contracts

Recommendations

• remove SafeMath from organization-contracts
  • remove SafeMath.sol from /contracts/lib
  • remove TestSafeMath.sol from /contracts/test/lib
  • remove import/using SafeMath from Organization.sol
  • remove safe_math.js from /test/lib
• remove SafeMath from Organization.sol in mosaic-contracts unless this is not necessary due to Use git-submodules to include UtilityToken and Organized contracts mosaic-contracts#712.
• establish and apply a policy for placement of organization contracts within repositories: under /contracts, /contracts/lib, or /contracts/organization?

[schemar]

Recommendations

• remove SafeMath from organization-contracts

absolutely agree 100%

• remove SafeMath from Organization.sol in mosaic-contracts unless this is not necessary due to OpenST/mosaic-contracts#712.

I would say that's redundant

• establish and apply a policy for placement of organization contracts within repositories: under /contracts, /contracts/lib, or /contracts/organization?

Or /contracts/lib/organization or /contracts/external/organization. I noticed that @pgev uses external directories for git submodules and I think he took that pattern from the gnosis safe.

[0xsarvesh]

Recommendations

• remove SafeMath from organization-contracts

absolutely agree 100%

• remove SafeMath from Organization.sol in mosaic-contracts unless this is not necessary due to OpenST/mosaic-contracts#712.

I would say that's redundant

• establish and apply a policy for placement of organization contracts within repositories: under /contracts, /contracts/lib, or /contracts/organization?

Or /contracts/lib/organization or /contracts/external/organization. I noticed that @pgev uses external directories for git submodules and I think he took that pattern from the gnosis safe.

We can reserve /contracts/external/something for git submodules which are outside OpenST network like genosis.
One more option in addition to /contracts/lib/organization can be /contracts/organization as contracts/lib can be used for solidity library. 🤔

Thoughts?

[jasonklein]

I think I second @sarvesh-ost re: reserving /contracts/external for non-OpenST work and /contracts/lib for library files.

With respect to where to place organizations contracts, if using git-submodule will add the whole directory, then presumably the naming should be the same (to minimize confusion), so <path to>/organization-contracts?

Or is it that the git-submodule work will include a script that takes the contracts and ditches the rest?

cc: @schemar

[schemar]

1. I agree that /contracts/lib/organization-contracts makes a lot of sense (as it is the repo name). I would say /contracts/lib/organization is also fine.
2. git submodules clone the entire tracked repository. And that is fine, as our repo will only track the reference to the remote repo.
3. While re-reading the submodules documentation, I also read about pinning the "version", I will add that to the PR now.

[schemar]

Oops, utility tokens doesn't have any releases or branches, yet 😅

Issue: it seems git submodules can only track branches, not tags. This may become a problem 🚨

[jasonklein]

I have added a ticket for removing SafeMath: #9

To settle the placement issue in other repositories:

• should the directory be organization or organization-contracts?
• should the directory be placed under /contracts or /contracts/lib?

I vote for organization-contracts and /contracts, so /contracts/organization-contracts

cc: @schemar, @sarvesh-ost

[jasonklein]

Also, on the topic of submodules: OpenST/developer-guidelines#20 (comment)

[deepesh-kn]

I have added a ticket for removing SafeMath: #9

To settle the placement issue in other repositories:

• should the directory be organization or organization-contracts?
• should the directory be placed under /contracts or /contracts/lib?

I vote for organization-contracts and /contracts, so /contracts/organization-contracts

I too vote for /contracts/organization-contracts

[jasonklein]

The open question (that did not really concern this ticket), was resolved with OpenST/utilitytoken-contracts#7 (/contracts/organization).

This ticket is thus completed.