Token SC is not compiling. about burner-wallet HOT 12 CLOSED

Also mentioned in issue #56

from burner-wallet.

Thanks @alexfisher , I didn't see the previous issue #56

from burner-wallet.

@riusricardo

Ah, sorry I don't think I got across what I meant to. You are totally right about the BalanceOf function and fix.

What I meant is that the DenDai contract currently in the BurnerWallet was put in master a bit prematurely I think. This normally wouldn't be too bad, but because the current test suite breaks if any one thing breaks, and the test suite is the current way to deploy contracts locally, it is causing these issues.

I was just suggesting not to worry too much about the specifics of that contract or implementation yet, as it is basically a quick implementation for testing UI. I'm working on the more final version of the SC here: Link

from burner-wallet.

Just a word of warning. The OZ libs that are now part of the repo are there for a very temporary fix. The ERC20 contract is not the standard one from OZ, but its modified to get the Burner/BuffiDai contracts to work.

We are going to be getting rid of those, adding in a more robust version of them which depends only on the standard versions of OZ, deleting the OZ contracts from the repo, and pulling them in only thru an npm dependency rather than an explicitly committed folder.

from burner-wallet.

Just a comment to the PR #56. Removing the private definition and not changing it to internal will default it to public. Probably not super insecure but there is a public function (balanceOf) to get the actual balance so there is no need to keep _balances as public.

from burner-wallet.

That contract really shouldn't even be checked into master tbh. Its a work in progress which should be in a branch, or at least not involved in the main test suite yet.

Don't worry about security or code for that contract because it doesn't look even remotely like it will once we actually get it to a production ready state.

If you are having test blocking issues I'd just suggest hacking around it to get whatever you need to get done. I'll get this taken care of today so it doesn't block people going forward.

from burner-wallet.

@dbe The contract is in production releases [email protected] (Solidity 0.4.24)
It is also available in the latest release [email protected] (Solidity 0.5.0)

from burner-wallet.

Closing because the OpenZeppelin libraries are now part of the repo. No more need to track this issue.

from burner-wallet.

I agree. But the fix breaks no compatibility with the ERC20 token. Changing to internal instead of private could be a permanent fix. OZ has that variable as private in new releases.
The ERC20 contract with the fix could be the only one that we should keep in the repo.

from burner-wallet.

With the new smart contract there is no need to have it be internal. We are using the contract APIs as intended with the new stuff.

from burner-wallet.

I'm not getting what you mean with "new smart contract". Are you working on a new PR?
If not, I'll take another look later and work in some upgrades.

from burner-wallet.

@dbe I see thew new contract branch. https://github.com/austintgriffith/burner-wallet/blob/new-vend/contracts/ERC20Vendable/ERC20Vendable.sol

from burner-wallet.