Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write to storage variable results in other field getting zero'd out #12

Closed
vgrichina opened this issue Mar 5, 2020 · 11 comments
Closed
Assignees

Comments

@vgrichina
Copy link

vgrichina commented Mar 5, 2020

PRs with relevant hacks:
#11
vgrichina/audius-protocol#1

To reproduce:

./node_modules/.bin/truffle test --network near test/userLibrary.js

notice zeros after userStorageRegistryKey:

internal call failed:  
  Offset  00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
00000000  08 C3 79 A0 00 00 00 00 00 00 00 00 00 00 00 00  .Ãy ............
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020  00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00  ... ............
00000030  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000040  00 00 00 55 61 64 64 55 73 65 72 32 2E 75 73 65  ...UaddUser2.use
00000050  72 53 74 6F 72 61 67 65 52 65 67 69 73 74 72 79  rStorageRegistry
00000060  4B 65 79 3A 00 00 00 00 00 00 00 00 00 00 00 00  Key:............
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000080  00 00 00 00 3A 00 00 00 00 00 00 00 00 00 00 00  ....:...........
00000090  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
000000A0  00 00 00 00                                      ....          

with commented out write to storage:
vgrichina/audius-protocol@33b0f9c

  Offset  00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
00000000  08 C3 79 A0 00 00 00 00 00 00 00 00 00 00 00 00  .Ãy ............
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020  00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00  ... ............
00000030  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000040  00 00 00 55 61 64 64 55 73 65 72 32 2E 75 73 65  ...UaddUser2.use
00000050  72 53 74 6F 72 61 67 65 52 65 67 69 73 74 72 79  rStorageRegistry
00000060  4B 65 79 3A 55 73 65 72 53 74 6F 72 61 67 65 00  Key:UserStorage.
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000080  00 00 00 00 3A 6F A3 CD 6C D6 07 3A 47 6C DD EC  ....:o£ÍlÖ.:GlÝì
00000090  C8 52 9B 0A 0D 01 56 A9 7F 00 00 00 00 00 00 00  ÈR�...V©.......
000000A0  00 00 00 00                                      ....            
@prestwich
Copy link
Contributor

Can you un-hardcode your account information from truffle-config and provide some more reproduction steps? What commands are you using to start a local node and deploy the EVM

@vgrichina
Copy link
Author

Can you un-hardcode your account information from truffle-config

Feel like it makes most sense if you do this, as I don't know what is the end result you want (like e.g. if I were you I'd just hardcode different info).

and provide some more reproduction steps?

Do you mean it doesn't reproduce with latest near-evm and ./node_modules/.bin/truffle test --network near test/userLibrary.js? What is the result you get?

Unfortunately I don't think I have commands I used to start node. I think I just used default local setup + increased some gas limit in config (as NEAR was previously returning error about gas limit).

@prestwich
Copy link
Contributor

prestwich commented May 21, 2020

steps I'm taking to try to reproduce:

audius-protocol branch: evm-bug-demo
near-web3-provider branch: evm-bug-demo
nearcore branch: stable
near-evm branch: master

modify audius truffle-config.js

-          fileKeyStore, ACCOUNT_ID, networkId, 'vg-evm');
+          fileKeyStore, ACCOUNT_ID, networkId, 'near_evm')

# start local test net
(nearcore) $ python scripts/start_unittest.py --local --release

# deploy near evm at account `near_evm`
(near-evm) $ cargo test

# create a new account named `vg-evm`
$ near create_account vg-evm --masterAccount test.near --nodeUrl http://localhost:3030 --keyPath nearcore/testdir/validator_key.json

# move keys to where audius truffle config expects
(audius-protocol/contracts) $ cp $NEARKEYS/default/vg-evm.json ./neardev/default 

# run with additional logging
(audius-protocol/contracts) $ truffle test --network near test/userLibrary.js --verbose-rpc

In order to run tests again, stop the node and repeat all steps

current status: hanging after solidity compilation. investigating more.

@prestwich
Copy link
Contributor

Update: checking out near-web3-provider branch fixes runs tests, which error with the following message:

    1) "before each" hook for "Should add one track save"


  0 passing (14s)
  1 failing

  1) Contract: UserLibrary
       "before each" hook for "Should add one track save":
     Error: invalid bytes value (arg="_subjectSig", coderType="bytes", value={})
      at Object.exports.addUserAndValidate (test/_lib/testUser.js:24:30)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (internal/process/task_queues.js:97:5)
      at Context.<anonymous> (test/userLibrary.js:75:5)

@prestwich
Copy link
Contributor

prestwich commented May 21, 2020

Okay. I've pushed the evm-bug-demo-plus-fixes branch. and I'm getting the following:

With AND without storage call

  Offset  00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
00000000  08 C3 79 A0 00 00 00 00 00 00 00 00 00 00 00 00  .Ãy ............
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020  00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00  ... ............
00000030  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000040  00 00 00 20 49 6E 76 61 6C 69 64 20 73 69 67 6E  ... Invalid sign
00000050  61 74 75 72 65 20 66 6F 72 20 67 69 76 65 6E 20  ature for given 
00000060  75 73 65 72                                      user            
    1) "before each" hook for "Should add one track save"


  0 passing (15s)
  1 failing

  1) Contract: UserLibrary
       "before each" hook for "Should add one track save":
     Error: Invalid txReceipt length
      at exports.parseTx (test/utils/parser.js:10:11)
      at exports.parseTxWithAssertsAndResp (test/utils/parser.js:32:14)
      at Object.exports.addUserAndValidate (test/_lib/testUser.js:26:3)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (internal/process/task_queues.js:97:5)
      at Context.<anonymous> (test/userLibrary.js:75:5)

Update: this was due to not porting the log translation from the bug-demo branch, and have extra json files in my keystore

@prestwich
Copy link
Contributor

as a sidenote I strongly disapprove of putting so much logic in the before each hook

@prestwich
Copy link
Contributor

Okay. I can reproduce:

  Offset  00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
00000000  08 C3 79 A0 00 00 00 00 00 00 00 00 00 00 00 00  .Ãy ............
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020  00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00  ... ............
00000030  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000040  00 00 00 55 61 64 64 55 73 65 72 31 2E 75 73 65  ...UaddUser1.use
00000050  72 53 74 6F 72 61 67 65 52 65 67 69 73 74 72 79  rStorageRegistry
00000060  4B 65 79 3A 55 73 65 72 53 74 6F 72 61 67 65 00  Key:UserStorage.
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000080  00 00 00 00 3A 6F A3 CD 6C D6 07 3A 47 6C DD EC  ....:o£ÍlÖ.:GlÝì
00000090  C8 52 9B 0A 0D 01 56 A9 7F 00 00 00 00 00 00 00  ÈR�...V©.......
000000A0  00 00 00 00                                      ....            
  Offset  00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
00000000  08 C3 79 A0 00 00 00 00 00 00 00 00 00 00 00 00  .Ãy ............
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020  00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00  ... ............
00000030  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000040  00 00 00 55 61 64 64 55 73 65 72 32 2E 75 73 65  ...UaddUser2.use
00000050  72 53 74 6F 72 61 67 65 52 65 67 69 73 74 72 79  rStorageRegistry
00000060  4B 65 79 3A 00 00 00 00 00 00 00 00 00 00 00 00  Key:............
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000080  00 00 00 00 3A 00 00 00 00 00 00 00 00 00 00 00  ....:...........
00000090  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
000000A0  00 00 00 00                                      ....          

@vgrichina can you check out the new branch and let me know if I'm missing anything, while I look into this more?

@prestwich
Copy link
Contributor

For future reference, here is the bytecode that produces the error message:

	0FAB    5B  JUMPDEST
	0FAC    60  PUSH1 0x02
	0FAE    60  PUSH1 0x01
	0FB0    14  EQ
	0FB1    60  PUSH1 0x05
	0FB3    54  SLOAD
	0FB4    60  PUSH1 0x06
	0FB6    60  PUSH1 0x00
	0FB8    90  SWAP1
	0FB9    54  SLOAD
	0FBA    90  SWAP1
	0FBB    61  PUSH2 0x0100
	0FBE    0A  EXP
	0FBF    90  SWAP1
	0FC0    04  DIV
	0FC1    73  PUSH20 0xffffffffffffffffffffffffffffffffffffffff
	0FD6    16  AND
	0FD7    60  PUSH1 0x40
	0FD9    51  MLOAD
	0FDA    60  PUSH1 0x20
	0FDC    01  ADD
	0FDD    80  DUP1
	0FDE    80  DUP1
	0FDF    7F  PUSH32 0x61646455736572322e7573657253746f7261676552656769737472794b65793a
	1000    81  DUP2
	1001    52  MSTORE
	1002    50  POP
	1003    60  PUSH1 0x20
	1005    01  ADD
	1006    83  DUP4
	1007    81  DUP2
	1008    52  MSTORE
	1009    60  PUSH1 0x20
	100B    01  ADD
	100C    80  DUP1
	100D    7F  PUSH32 0x3a00000000000000000000000000000000000000000000000000000000000000
	102E    81  DUP2
	102F    52  MSTORE
	1030    50  POP
	1031    60  PUSH1 0x01
	1033    01  ADD
	1034    82  DUP3
	1035    73  PUSH20 0xffffffffffffffffffffffffffffffffffffffff
	104A    16  AND
	104B    73  PUSH20 0xffffffffffffffffffffffffffffffffffffffff
	1060    16  AND
	1061    60  PUSH1 0x60
	1063    1B  SHL
	1064    81  DUP2
	1065    52  MSTORE
	1066    60  PUSH1 0x14
	1068    01  ADD
	1069    92  SWAP3
	106A    50  POP
	106B    50  POP
	106C    50  POP
	106D    60  PUSH1 0x40
	106F    51  MLOAD
	1070    60  PUSH1 0x20
	1072    81  DUP2
	1073    83  DUP4
	1074    03  SUB
	1075    03  SUB
	1076    81  DUP2
	1077    52  MSTORE
	1078    90  SWAP1
	1079    60  PUSH1 0x40
	107B    52  MSTORE
	107C    90  SWAP1
	107D    61  PUSH2 0x1121
	1080    57  *JUMPI

And here is my hand-annotated decompiled code:

// load the free mem pointer. 
// We will start writing 0x20 bytes after it.
// This leaves space for the solidity bytearray length prefix
var startWritePointer = memory[0x40:0x60] + 0x20;

// write the raw string "addUser2.userStorageRegistryKey":
memory[startWritePointer:startWritePointer + 0x20] = 0x61646455736572322e7573657253746f7261676552656769737472794b65793a;
nextWrite = startWritePointer + 0x20;

// userStorageRegistryKey
memory[nextWrite:nextWrite + 0x20] = storage[0x05];
nextWrite += 0x20;

// write the raw string "":""
memory[nextWrite:nextWrite + 0x20] = 0x3a00000000000000000000000000000000000000000000000000000000000000;
nextWrite += 0x01;

// verifierAddress 
// It gets shifted right by 12 bytes. normally the address is in the low 20 bytes, we want it in the high 20
memory[nextWrite:nextWrite + 0x20] = (storage[0x06] & 0xffffffffffffffffffffffffffffffffffffffff) << 0x60;

// determine where the string ends
var stringBodyEnd = nextWrite + 0x14;

// determine where the length prefix ought to go
var freeMemPointer = memory[0x40:0x60];

// write the length prefix
memory[freeMemPointer:freeMemPointer + 0x20] = stringBodyEnd - freeMemPointer - 0x20;

// Update the free mem pointer to point at free space after the string.
memory[0x40:0x60] = stringBodyEnd;

@prestwich
Copy link
Contributor

Alright. I can confirm that the refactor will fix this

Offset  00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
00000000  08 C3 79 A0 00 00 00 00 00 00 00 00 00 00 00 00  .Ãy ............
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020  00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00  ... ............
00000030  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000040  00 00 00 55 61 64 64 55 73 65 72 32 2E 75 73 65  ...UaddUser2.use
00000050  72 53 74 6F 72 61 67 65 52 65 67 69 73 74 72 79  rStorageRegistry
00000060  4B 65 79 3A 55 73 65 72 53 74 6F 72 61 67 65 00  Key:UserStorage.
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000080  00 00 00 00 3A 6F A3 CD 6C D6 07 3A 47 6C DD EC  ....:o£ÍlÖ.:GlÝì
00000090  C8 52 9B 0A 0D 01 56 A9 7F 00 00 00 00 00 00 00  ÈR�...V©.......
000000A0  00 00 00 00                                      ....    

@ewilz
Copy link
Contributor

ewilz commented Jul 1, 2020

Can this issue be closed?

@prestwich
Copy link
Contributor

Yes it can be :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants