Optimize LoadAccount #2693

BeniaminDrasovean · 2021-01-13T15:37:15Z

Separated getCode from loadAccount - code is only got from trie when it is needed.
Added fixes on esdt callbacks and return of values
Integrated new arwen

iulianpascalau · 2021-01-13T16:18:28Z

data/state/accountsDB.go

+	}
+
+	if len(baseAcc.GetCode()) != 0 {
+		return baseAcc.GetCode()


assuming the base account has a code inside and returning it in that case might lead to this scenario:
I have an account, I've put by mistake something in its code field and then I call AccountsDB.GetCode assuming I will get the "real" contract code. Instead I get the existing byte slice from the user account.

Should I retrieve the code from the trie each time?

I think it is better without this sort of cache (we can do it in other parts of the code). In this way this function express the intent in a more straight forward way.

you can get it - as we have extra gas cost for it now.

data/state/accountsDB.go

data/state/baseAccount.go

iulianpascalau · 2021-01-13T16:22:05Z

data/state/baseAccount.go

@@ -22,6 +23,7 @@ func (ba *baseAccount) GetCode() []byte {

 // SetCode sets the actual code that needs to be run in the VM
 func (ba *baseAccount) SetCode(code []byte) {
+	ba.hasNewCode = true


ba.hasNewCode = !bytes.Equal(code, ba.code) ?

If the code is nil, and the old code was not loaded from the trie (ba.code == nil), this will set false, and the code would not be removed.

epochStart/bootstrap/disabled/disabledAccountsAdapter.go

node/node.go

node/txsimulator/wrappedAccountsDB.go

sasurobert

I think you should delete GetCode from the UserAccountsHandler interface

…turn esdt

# Conflicts: # data/state/export_test.go

…ize-LoadAccount # Conflicts: # go.mod # go.sum

# Conflicts: # go.mod # go.sum

…vert Fix esdt cross sc fail revert

iulianpascalau · 2021-01-14T11:38:17Z

integrationTests/vm/arwen/arwenVM/arwenVM_test.go

@@ -246,8 +246,8 @@ func TestMultipleTimesERC20BigIntInBatches(t *testing.T) {
 	if testing.Short() {
 		t.Skip("this is not a short test")
 	}
-	gasSchedule, _ := core.LoadGasScheduleConfig("../../../../cmd/node/config/gasSchedules/gasScheduleV2.toml")
-	deployAndExecuteERC20WithBigInt(t, 3, 1000, gasSchedule, "../testdata/erc20-c-03/wrc20_arwen.wasm", "transferToken", false)
+	//gasSchedule, _ := core.LoadGasScheduleConfig("../../../../cmd/node/config/gasSchedules/gasScheduleV2.toml")


remove commented code

iulianpascalau · 2021-01-14T11:39:04Z

node/node.go

@@ -1124,7 +1124,8 @@ func (n *Node) GetAccount(address string) (state.UserAccountHandler, error) {
 	accWrp, err := n.accounts.GetExistingAccount(addr)
 	if err != nil {
 		if err == state.ErrAccNotFound {
-			return state.NewUserAccount(addr)
+			newAcc, err := state.NewUserAccount(addr)


return directly?

SebastianMarian · 2021-01-14T12:13:58Z

data/state/export_test.go

@@ -25,6 +25,10 @@ func (adb *AccountsDB) GetObsoleteHashes() map[string][][]byte {
 	return adb.obsoleteDataTrieHashes
 }

+func GetCode(account baseAccountHandler) []byte {


GetCodeHash?

this is an export function only. some stuff is mocked for tests only.

Yes it is, but it returns a code hash and not the code, the name is misleading also for tests :)

LucianMincu

System tests passed.

sasurobert and others added 2 commits January 13, 2021 17:24

esdt cross shard revert fix with sc call

ea43a3c

add getCode to accountsDb

a8485bb

BeniaminDrasovean changed the title ~~add getCode to accountsDb~~ Optimize LoadAccount Jan 13, 2021

sasurobert self-requested a review January 13, 2021 16:01

iulianpascalau reviewed Jan 13, 2021

View reviewed changes

BeniaminDrasovean and others added 2 commits January 13, 2021 18:58

fix after review and small changes

1b0e952

small fix on process

5dd172c

iulianpascalau previously approved these changes Jan 13, 2021

View reviewed changes

sasurobert and others added 2 commits January 13, 2021 19:29

fix return only esdt transfer in case of error - callback

ac44331

Merge branch 'development' into optimize-LoadAccount

e87e4ab

sasurobert requested changes Jan 13, 2021

View reviewed changes

sasurobert and others added 2 commits January 13, 2021 20:59

fix asyncCall back on esdt transfer

8d329a2

fix after review

f7adf07

BeniaminDrasovean dismissed iulianpascalau’s stale review via f7adf07 January 13, 2021 19:37

sasurobert and others added 3 commits January 13, 2021 21:47

it was a double resolve of the same problem - creating 2 scrs with re…

33e628d

…turn esdt

Merge branch 'development' into fix-esdt-cross-sc-fail-revert

d452fca

Merge branch 'development' into fix-esdt-cross-sc-fail-revert

3d799a4

sasurobert previously approved these changes Jan 13, 2021

View reviewed changes

sasurobert and others added 3 commits January 13, 2021 22:48

go mod change

b7a482e

Merge branch 'development' into fix-esdt-cross-sc-fail-revert

92e5319

small fixes

0cce94f

BeniaminDrasovean dismissed sasurobert’s stale review via 0cce94f January 14, 2021 09:15

BeniaminDrasovean and others added 8 commits January 14, 2021 11:16

Merge branch 'development' into optimize-LoadAccount

78c5c61

# Conflicts: # data/state/export_test.go

go mod change

8458fe9

Merge remote-tracking branch 'origin/optimize-LoadAccount' into optim…

646ddb2

…ize-LoadAccount # Conflicts: # go.mod # go.sum

fixing tests

b505d3a

refactor GetCode

cb252a1

refactor of refactor

80fe266

new arwen

b928057

Merge branch 'optimize-LoadAccount' into fix-esdt-cross-sc-fail-revert

8bc2b7b

# Conflicts: # go.mod # go.sum

sasurobert added 3 commits January 14, 2021 13:15

fix after merge

e3d492f

Merge pull request #2697 from ElrondNetwork/fix-esdt-cross-sc-fail-re…

e8e1b53

…vert Fix esdt cross sc fail revert

fix some tests

1033733

sasurobert marked this pull request as ready for review January 14, 2021 11:31

iulianpascalau requested changes Jan 14, 2021

View reviewed changes

fix after review

e9594a6

iulianpascalau approved these changes Jan 14, 2021

View reviewed changes

SebastianMarian reviewed Jan 14, 2021

View reviewed changes

sasurobert approved these changes Jan 14, 2021

View reviewed changes

SebastianMarian approved these changes Jan 14, 2021

View reviewed changes

LucianMincu approved these changes Jan 14, 2021

View reviewed changes

LucianMincu merged commit d726d38 into development Jan 14, 2021

LucianMincu deleted the optimize-LoadAccount branch January 14, 2021 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize LoadAccount #2693

Optimize LoadAccount #2693

BeniaminDrasovean commented Jan 13, 2021 •

edited by sasurobert

iulianpascalau Jan 13, 2021

BeniaminDrasovean Jan 13, 2021

iulianpascalau Jan 13, 2021

sasurobert Jan 13, 2021

iulianpascalau Jan 13, 2021

BeniaminDrasovean Jan 13, 2021 •

edited

sasurobert left a comment

iulianpascalau Jan 14, 2021

sasurobert Jan 14, 2021

iulianpascalau Jan 14, 2021

sasurobert Jan 14, 2021

SebastianMarian Jan 14, 2021

sasurobert Jan 14, 2021

SebastianMarian Jan 14, 2021

LucianMincu left a comment

Optimize LoadAccount #2693

Optimize LoadAccount #2693

Conversation

BeniaminDrasovean commented Jan 13, 2021 • edited by sasurobert

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

BeniaminDrasovean Jan 13, 2021 • edited

Choose a reason for hiding this comment

sasurobert 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

Choose a reason for hiding this comment

LucianMincu left a comment

Choose a reason for hiding this comment

BeniaminDrasovean commented Jan 13, 2021 •

edited by sasurobert

BeniaminDrasovean Jan 13, 2021 •

edited