SecurityToken decimals are hardcoded to 0

[pabloruiz55]

Decimals Should be part of the constructor parameters

[satyamakgec]

@everhusk can you tell me security tokens are decimals based or not

[AdamDanielKing]

I remember it being a design decision to only have whole-numbered security tokens. The reason the decimals property exists at all is that it's part of ERC20.

[everhusk]

Yeah that's correct, 0 decimals for security tokens was decided to mimic how shares exist today.

[pabloruiz55]

Under which circumstances would it make sense to have divisible tokens?

@adamdossa says:

Main reason I think would be liquidity. i.e. an organisation issues an ST, at an initial price of $1. After doing very well, the token price is now $100. Without the token being divisible it wouldn't be possible to invest an amount under $100, which may hurt the market. In equities stocks are often split in these circumstances, and I feel the equivalent in the blockchain world is divisible tokens.

[everhusk]

Hmm, okay let's pull this out to the application level. We'll hardcode in polymath.js.

[pabloruiz55]

So... Adding yet another constructor parameter to SecurityToken and SecurityTokenRegistrar causes the "InternalCompilerError: Stack too deep, try removing local variables." error.
How do you suggest we fix this? @everhusk @satyamakgec @adamdossa

[pabloruiz55]

There's one way to make it work, which is packing a few of the variables that share the same type into an array and passing that around.
That works, but will require a lot of refactoring all around, modifying tests and whatever is already done in the front-end.

@everhusk let me know if it worth it or we should push this to V2.

[adamdossa]

Hey @pabloruiz55 - I just tried this in the decimals_fix branch, and compilation worked fine for me...

Could you let me know how your changes differ to mine?

https://github.com/PolymathNetwork/polymath-core/tree/decimals-fix

Adams-MBP:polymath-core adamdossa$ truffle compile
Compiling ./contracts/STO20.sol...
Compiling ./contracts/SafeMath.sol...
Compiling ./contracts/SecurityToken.sol...
Compiling ./contracts/interfaces/ICompliance.sol...
Compiling ./contracts/interfaces/ICustomers.sol...
Compiling ./contracts/interfaces/IERC20.sol...
Compiling ./contracts/interfaces/ISTRegistrar.sol...
Compiling ./contracts/interfaces/ITemplate.sol...
Writing artifacts to ./build/contracts

[pabloruiz55]

@adamdossa, after fixing a bug (missing the _decimals declaration on the interface file), when trying to compile it gives me the same error I was getting with my version:

InternalCompilerError: Stack too deep, try removing local variables.
Compilation failed. See above.

Also, your code is missing a few things that we need to do with the decimals variable, but I guess you didn't include that just to try this out.

[adamdossa]

@pabloruiz55 I just fixed the decimals-fix branch and it is now compiling without a hardcoded decimals variable.
There would need to be more changes as you say, but just wanted to demonstrate splitting out the function to avoid the "Stack too deep" compilation issue as a first step.
If you're able to compile this branch, I will then fix up the rest of the code and present it as a PR for review.

[pabloruiz55]

@adamdossa great! It does compile and migrate now. Awesome. I didn't think that just moving the code to another function would fix that error even when the function is called within the first function.
Are you adding the rest of the code?

[adamdossa]

Yep - will do so shortly on this branch.