global settings role to burn for all #4180

sasurobert · 2022-06-08T13:14:16Z

global settings role to burn for all

codecov-commenter · 2022-06-08T13:26:18Z

Codecov Report

Merging #4180 (2357306) into rc/2022-july (779ac14) will increase coverage by 0.00%.
The diff coverage is 84.72%.

@@              Coverage Diff              @@
##           rc/2022-july    #4180   +/-   ##
=============================================
  Coverage         73.61%   73.62%           
=============================================
  Files               645      645           
  Lines             85227    85299   +72     
=============================================
+ Hits              62744    62804   +60     
- Misses            17809    17815    +6     
- Partials           4674     4680    +6

Impacted Files	Coverage Δ
vm/systemSmartContracts/esdt.go	`83.72% <84.72%> (+0.05%)`	⬆️
storage/txcache/txListBySenderMap.go	`97.50% <0.00%> (-2.50%)`	⬇️
p2p/libp2p/netMessenger.go	`76.07% <0.00%> (-0.27%)`	⬇️
debug/resolver/interceptorResolver.go	`79.11% <0.00%> (+1.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 779ac14...2357306. Read the comment docs.

iulianpascalau · 2022-06-16T12:43:01Z

integrationTests/vm/esdt/roles/esdtRoles_test.go

@@ -5,6 +5,7 @@ package roles

 import (
 	"encoding/hex"
+	"github.com/ElrondNetwork/elrond-go/config"


iulianpascalau · 2022-06-16T12:48:39Z

vm/systemSmartContracts/esdt.go

+
+	burnForAllExists := checkIfDefinedRoleExistsInToken(token, []byte(vmcommon.ESDTRoleBurnForAll))
+	if burnForAllExists {
+		e.eei.AddReturnMessage("cannot set burn  for all as it was already set")


there are 2 white spaces in the message between burn and for. Please remove one.

# Conflicts: # go.mod # go.sum

bogdan-rosianu · 2022-06-16T14:10:23Z

vm/systemSmartContracts/esdt_test.go

+
+	vmInput.CallerAddr = owner
+	vmInput.Arguments = [][]byte{tokenName}
+	output = e.Execute(vmInput)


linter: ineffectual assignment to output (ineffassign)

bogdan-rosianu · 2022-06-16T14:13:57Z

vm/systemSmartContracts/esdt.go

@@ -2050,6 +2160,9 @@ func (e *esdt) EpochConfirmed(epoch uint32, _ uint64) {

 	e.flagRegisterAndSetAllRoles.SetValue(epoch >= e.registerAndSetAllRolesEnableEpoch)
 	log.Debug("ESDT register and set all roles", "enabled", e.flagRegisterAndSetAllRoles.IsSet())
+
+	e.flagBurnForAll.SetValue(epoch >= e.burnForAllEnableEpoch)
+	log.Debug("ESDT register and set all roles", "enabled", e.flagBurnForAll.IsSet())


wrong log
log.Debug("ESDT burn for all", "enabled", e.flagBurnForAll.IsSet()) *

bogdan-rosianu · 2022-06-16T14:15:30Z

vm/systemSmartContracts/esdt.go

+	return vmcommon.Ok
+}
+
+func (e *esdt) setBurnForAll(args *vmcommon.ContractCallInput) vmcommon.ReturnCode {


can you rename it to setBurnRoleForAll ?

or setBurnRoleGlobally

bogdan-rosianu · 2022-06-16T14:15:55Z

vm/systemSmartContracts/esdt.go

+
+	burnForAllExists := checkIfDefinedRoleExistsInToken(token, []byte(vmcommon.ESDTRoleBurnForAll))
+	if burnForAllExists {
+		e.eei.AddReturnMessage("cannot set burn for all as it was already set")


cannot set burn for all role as it was already set

bogdan-rosianu · 2022-06-16T14:16:44Z

vm/systemSmartContracts/esdt.go

+
+	burnForAllExists := checkIfDefinedRoleExistsInToken(token, []byte(vmcommon.ESDTRoleBurnForAll))
+	if !burnForAllExists {
+		e.eei.AddReturnMessage("cannot un set burn for all as it was not set")


cannot unset burn for all role as it was not set

gabi-vuls

System test passed.
@@ Log scanner @@

burn-for-all

================================================================================

Known Warnings 14
New Warnings 4
Known Errors 0
New Errors 0
Panics 0
================================================================================

gabi-vuls

System test passed.

# Conflicts: # go.mod # go.sum # vm/systemSmartContracts/esdt.go

…on_arwen # Conflicts: # go.mod # go.sum

…n_arwen

Updated arwen and vm common

iulianpascalau · 2022-06-30T08:08:43Z

vm/systemSmartContracts/esdt.go

@@ -131,6 +133,7 @@ func NewESDTSmartContract(args ArgsNewESDTSmartContract) (*esdt, error) {
 		nftCreateONMultiShardEnableEpoch:  args.EpochConfig.EnableEpochs.ESDTNFTCreateOnMultiShardEnableEpoch,
 		metaESDTEnableEpoch:               args.EpochConfig.EnableEpochs.MetaESDTSetEnableEpoch,
 		registerAndSetAllRolesEnableEpoch: args.EpochConfig.EnableEpochs.ESDTRegisterAndSetAllRolesEnableEpoch,
+		checkMetaESDTOnRolesEnableEpoch:   args.EpochConfig.EnableEpochs.ManagedCryptoAPIsEnableEpoch,


we need a new flag for this

this is not the change - line 135 is the change - github is showing it wrong

bogdan-rosianu · 2022-06-30T08:08:15Z

vm/systemSmartContracts/esdt.go

-		return e.unSetBurnForAll(args)
+	case "setBurnRoleGlobally":
+		return e.setBurnRoleGlobally(args)
+	case "unsetBurnRoleGlobally":


BUG: add return e.unsetBurnRoleGlobally(args)

nevermind, only Github display issue

bogdan-rosianu · 2022-06-30T08:09:34Z

vm/systemSmartContracts/esdt.go

@@ -2177,8 +2181,11 @@ func (e *esdt) EpochConfirmed(epoch uint32, _ uint64) {
 	e.flagRegisterAndSetAllRoles.SetValue(epoch >= e.registerAndSetAllRolesEnableEpoch)
 	log.Debug("ESDT register and set all roles", "enabled", e.flagRegisterAndSetAllRoles.IsSet())

+	e.flagCheckMetaESDTOnRolesEnableEpoch.SetValue(epoch >= e.checkMetaESDTOnRolesEnableEpoch)


where is this flag used?

again - github is stupid. this is already in on master.

gabi-vuls

System test passed.

global settings role to burn for all

1078204

sasurobert added 4 commits June 9, 2022 11:52

global settings role to burn for all

6d5eaa2

added integration tests

6636255

fixing tests

b6dee94

linter fixes

e9b2283

iulianpascalau self-requested a review June 16, 2022 12:42

dorin-iancu previously approved these changes Jun 16, 2022

View reviewed changes

iulianpascalau reviewed Jun 16, 2022

View reviewed changes

fix after review

cf7306e

sasurobert dismissed dorin-iancu’s stale review via cf7306e June 16, 2022 13:07

sasurobert added 3 commits June 16, 2022 16:08

Merge branch 'development' into burn-for-all

95221ef

# Conflicts: # go.mod # go.sum

fix after review

64563c5

proper release

9f20ddf

bogdan-rosianu reviewed Jun 16, 2022

View reviewed changes

gabi-vuls previously approved these changes Jun 17, 2022

View reviewed changes

fix after review

4b891b6

sasurobert dismissed gabi-vuls’s stale review via 4b891b6 June 20, 2022 07:11

gabi-vuls previously approved these changes Jun 20, 2022

View reviewed changes

dorin-iancu previously approved these changes Jun 20, 2022

View reviewed changes

iulianpascalau previously approved these changes Jun 20, 2022

View reviewed changes

sstanculeanu and others added 4 commits June 30, 2022 10:21

updated arwen and vm common

fe6d566

fixed tests

09d914e

Merge branch 'development' into burn-for-all

2701f4e

# Conflicts: # go.mod # go.sum # vm/systemSmartContracts/esdt.go

new go mod

e98089e

sasurobert dismissed stale reviews from iulianpascalau, dorin-iancu, and gabi-vuls via e98089e June 30, 2022 07:42

sstanculeanu and others added 2 commits June 30, 2022 10:44

Merge remote-tracking branch 'origin/burn-for-all' into update_vmcomm…

51db334

…on_arwen # Conflicts: # go.mod # go.sum

Merge branch 'development' into burn-for-all

e18f025

sasurobert changed the base branch from development to rc/2022-july June 30, 2022 07:45

sasurobert and others added 5 commits June 30, 2022 10:46

new go mod

8425ea0

Merge remote-tracking branch 'origin/development' into update_vmcommo…

42938e7

…n_arwen

go mod tidy

b89fade

Merge branch 'burn-for-all' into update_vmcommon_arwen

5b303a7

Merge pull request #4244 from ElrondNetwork/update_vmcommon_arwen

2357306

Updated arwen and vm common

iulianpascalau reviewed Jun 30, 2022

View reviewed changes

bogdan-rosianu requested changes Jun 30, 2022

View reviewed changes

bogdan-rosianu approved these changes Jun 30, 2022

View reviewed changes

iulianpascalau approved these changes Jun 30, 2022

View reviewed changes

gabi-vuls approved these changes Jul 1, 2022

View reviewed changes

gabi-vuls merged commit 8c74007 into rc/2022-july Jul 1, 2022

gabi-vuls deleted the burn-for-all branch July 1, 2022 15:49

iulianpascalau added area:processing impact:medium interested:dex-frontend interested:dex-backend interested:dapp-sdks interested:sc interested:protocol type:improvement complexity:medium labels Aug 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

global settings role to burn for all #4180

global settings role to burn for all #4180

sasurobert commented Jun 8, 2022

codecov-commenter commented Jun 8, 2022 •

edited

iulianpascalau Jun 16, 2022

iulianpascalau Jun 16, 2022

sasurobert Jun 16, 2022

bogdan-rosianu Jun 16, 2022

sasurobert Jun 20, 2022

bogdan-rosianu Jun 16, 2022

sasurobert Jun 20, 2022

bogdan-rosianu Jun 16, 2022

bogdan-rosianu Jun 16, 2022

sasurobert Jun 20, 2022

bogdan-rosianu Jun 16, 2022

sasurobert Jun 20, 2022

bogdan-rosianu Jun 16, 2022

sasurobert Jun 20, 2022

gabi-vuls left a comment

gabi-vuls left a comment

iulianpascalau Jun 30, 2022

sasurobert Jun 30, 2022

bogdan-rosianu Jun 30, 2022

bogdan-rosianu Jun 30, 2022

bogdan-rosianu Jun 30, 2022

sasurobert Jun 30, 2022

gabi-vuls left a comment

global settings role to burn for all #4180

global settings role to burn for all #4180

Conversation

sasurobert commented Jun 8, 2022

codecov-commenter commented Jun 8, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabi-vuls left a comment

Choose a reason for hiding this comment

burn-for-all

gabi-vuls left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabi-vuls left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 8, 2022 •

edited