Skip to content
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

Freeze Minting #224

Merged
merged 6 commits into from Aug 30, 2018
Merged

Freeze Minting #224

merged 6 commits into from Aug 30, 2018

Conversation

thegostep
Copy link

@thegostep thegostep commented Aug 28, 2018

Requirements

  • Issuer MUST be able to call a function which prevents them from minting tokens in the future
  • This issue MUST be disabled by default until enabled by Polymath
  • Function MUST emit event notifying minting is done
  • Issuer MUST be able to mint tokens at any time before calling this function
  • Issuer MUST NOT be able to mint tokens at any time after calling this function

Changes

ISecurityToken.sol

  • remove function finishMintingIssuer() and function finishMintingSTO()
  • add function freezeMinting()

SecurityToken.sol

  • rename bool freeze to bool transfersFrozen
  • add bool mintingFrozen
  • remove bool finishedIssuerMinting and bool finishedSTOMinting
  • remove event LogFinishMintingSTO and event LogFinishMintingIssuer
  • add event LogFreezeMinting
  • remove function finishMintingIssuer() and function finishMintingSTO()
  • add function freezeMinting()
  • change modifier isMintingAllowed to only check for mintingFrozen
  • change modifier onlyModule() to remove fallback on onlyOwner
  • add modifier onlyModuleOrOwner()

create IFeatureRegisty.sol
create FeatureRegisty.sol

RegistryUpdater.sol

  • add featureRegistry

@thegostep thegostep changed the base branch from master to development-1.5.0 August 28, 2018 05:18
@pabloruiz55
Copy link
Contributor

What's the reasoning behind putting the Polymath allowFreezeMinting switch in the ModuleRegistry? Not that any of the other registries make more sense, though.
Perhaps what's more sensible to do is to create a new Polymath owned registry where all these regulatory feature switches go in, rather. @thegostep

@thegostep
Copy link
Author

@pabloruiz55 The reason for putting the feature switch in the MR is because it is the only registry with a link to the deployed ST since they communicate for module deployment. WRT a new feature switch contract, it seems like a bit of an overkill and would create additional dependencies. I am still not familiar with the new upgradability pattern so not sure what will be the implications. Maybe @adamdossa has more thoughts.

@pabloruiz55
Copy link
Contributor

@thegostep WRT where this switch goes, let's keep it simple for now:

  1. Create new registry contract and move the logic added to the MR there.
  2. Add that contract to the PolymathRegistry as a new key
  3. From the SecurityToken contract you can access the new contract through the PolymathRegistry and get the flag.

@thegostep thegostep merged commit bb50297 into development-1.5.0 Aug 30, 2018
@pabloruiz55 pabloruiz55 deleted the freeze-minting branch September 3, 2018 11:39
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.

None yet

3 participants