En 7630/esdt burn mint freeze #2328
Conversation
…nto EN-7630/ESDT-burn-mint-freeze
| @@ -439,6 +439,21 @@ const BuiltInFunctionSaveKeyValue = "SaveKeyValue" | |||
| // BuiltInFunctionESDTTransfer is the key for the elrond standard digital token transfer built-in function | |||
| const BuiltInFunctionESDTTransfer = "ESDTTransfer" | |||
|
|
|||
| // BuiltInFunctionESDTFreeze is the key for the elrond standard digital token freeze built-in function | |||
| const BuiltInFunctionESDTFreeze = "ESDTFreeze" | |||
There was a problem hiding this comment.
Do we want function names to start with uppercase?
There was a problem hiding this comment.
we already have it for ESDTTransfer - we might change them - it will need change in transaction generator testing component as well.
There was a problem hiding this comment.
I would use esdtFreeze style for all, but we can change this later
vm/systemSmartContracts/eei.go
Outdated
| @@ -136,6 +136,11 @@ func (host *vmContext) GetBalance(addr []byte) *big.Int { | |||
| return account.GetBalance() | |||
| } | |||
|
|
|||
| // TransferToAll handles transfering an information to all the shards | |||
| func (host *vmContext) TransferToAll(sender []byte, input []byte) { | |||
There was a problem hiding this comment.
I think this would require a different name since it looks like it has something to do with the value transfer function below
There was a problem hiding this comment.
renamed to SendGlobalSettingToAll
vm/systemSmartContracts/esdt.go
Outdated
| @@ -101,19 +103,23 @@ func (e *esdt) Execute(args *vmcommon.ContractCallInput) vmcommon.ReturnCode { | |||
| case "mint": | |||
| return e.mint(args) | |||
| case "freeze": | |||
| return e.freeze(args) | |||
| return e.freezeUnFreeze(args, core.BuiltInFunctionESDTFreeze) | |||
There was a problem hiding this comment.
toggleFreeze and togglePause?
vm/systemSmartContracts/esdt.go
Outdated
| optionalArg := string(arg) | ||
| switch optionalArg { | ||
| case burnable: | ||
| token.Burnable = true |
There was a problem hiding this comment.
Can't we also unset a property?
vm/systemSmartContracts/esdt.go
Outdated
| } | ||
| burntValue := big.NewInt(0).SetBytes(args.Arguments[1]) | ||
| if burntValue.Cmp(big.NewInt(0)) <= 0 { | ||
| e.eei.AddReturnMessage(vm.ErrNegativeOrZeroInitialSupply.Error()) |
There was a problem hiding this comment.
Please add another error, this would be a bit misleading
| e.eei.AddReturnMessage("token is not burnable") | ||
| return vmcommon.UserError | ||
| } | ||
| token.BurntValue.Add(token.BurntValue, burntValue) |
There was a problem hiding this comment.
Shouldn't we actually transfer this value to the burn address from the caller's balance?
There was a problem hiding this comment.
this value is actually transferred to the meta and deleted here - full implementation is not done. This is only a part. Any sender can BURN separately tokens through sending to burn address.
vm/systemSmartContracts/esdt.go
Outdated
| return vmcommon.UserError | ||
| } | ||
| if !token.Mintable { | ||
| e.eei.AddReturnMessage("token is not burnable") |
There was a problem hiding this comment.
"token is not mintable"
| //TODO: implement me | ||
| return vmcommon.Ok | ||
| func (e *esdt) basicOwnerChecks(args *vmcommon.ContractCallInput) (*ESDTData, vmcommon.ReturnCode) { | ||
| if args.CallValue.Cmp(zero) != 0 { |
There was a problem hiding this comment.
I don't think this check should be in basicOwnershipChecks. Or better so we don't have to figure out how to refactor, we can rename this method
vm/systemSmartContracts/eei.go
Outdated
| @@ -136,6 +136,11 @@ func (host *vmContext) GetBalance(addr []byte) *big.Int { | |||
| return account.GetBalance() | |||
| } | |||
|
|
|||
| // TransferToAll handles transfering an information to all the shards | |||
| func (host *vmContext) TransferToAll(sender []byte, input []byte) { | |||
|
|
|||
There was a problem hiding this comment.
Missing implementation?
| e.eei.AddReturnMessage("destination address of invalid length") | ||
| return vmcommon.UserError | ||
| } | ||
| destination = args.Arguments[2] |
There was a problem hiding this comment.
why do we need the argument for the destination? Why not mint function should always sent tokens on the args.CallerAddr directly since it is the owner of the esdt token?
There was a problem hiding this comment.
You might want to send to a new contract or to a new investor.
vm/systemSmartContracts/esdt.go
Outdated
| func (e *esdt) burn(_ *vmcommon.ContractCallInput) vmcommon.ReturnCode { | ||
| //TODO: implement me | ||
| return vmcommon.Ok | ||
| func (e *esdt) upgradeProperties(token *ESDTData, args [][]byte) { |
There was a problem hiding this comment.
will come in next PRs. Want to write full implementation and happy path integration test first.
| } | ||
|
|
||
| func checkAndGetSetting(arg string) (bool, error) { | ||
| if arg == "true" { |
There was a problem hiding this comment.
Might use something like:
if strings.ToLower(arg) == "true" {.....
if strings.ToLower(arg) == "false" {.as to work regardless how the caps were used
There was a problem hiding this comment.
wouldn't change it - it has to be strict.
| case upgradable: | ||
| token.Upgradable = true | ||
| token.Upgradable = val |
There was a problem hiding this comment.
This has to be treated cautiously by the the owner of the esdt token. He/she might wrongly set upgradable = false with some unwanted settings that will then become impossible to be changed in the future.
There was a problem hiding this comment.
this is the same with upgradable contract. by default upgradable is true.
First PR of implementing the new functions in the system smart contract.