initial support for mapping types #498

cdetrio · 2017-05-02T16:22:04Z

Work in progress, don't merge.

yann300 · 2017-05-04T07:20:58Z

cool!!

cdetrio · 2017-05-04T17:27:23Z

@yann300 btw feel free to review and intervene early. Particularly in src/ui/SolidityState.js and the call to storageViewer.storageRange, that will probably get refactored to support getting preimages using debug_preimage (see ethereum/go-ethereum#3543 and ethereum/go-ethereum#3407).

cdetrio · 2017-05-18T15:26:07Z

src/solidity/decodeInfo.js

@@ -233,7 +233,7 @@ function getEnum (type, stateDefinitions, contractName) {
  * @param {String} location - location of the data (storage ref| storage pointer| memory| calldata)
  * @return {Array} containing all members of the current struct type
  */
-function getStructMembers (type, stateDefinitions, contractName) {
+function getStructMembers (type, stateDefinitions, contractName, location) {


the location argument is never used in the getStructMembers function because to get the members only the struct definition is used (not the stateVar declaration, which does have a storage slot location).

cdetrio · 2017-05-18T15:47:53Z

src/solidity/types/StringType.js

@@ -40,7 +40,7 @@ function format (decoded) {
  var value = decoded.value
  value = value.replace('0x', '').replace(/(..)/g, '%$1')
  var ret = {
-    // length: decoded.length, // unneeded, only dynamicBytes uses length
+    length: decoded.length,


I can't remember why i was convinced that the length here is never used, but it doesn't matter.

yann300 · 2017-05-18T15:51:48Z

yeah the length here is not really relevant. but removing this breaks tests

chriseth · 2017-05-22T12:40:26Z

src/solidity/decodeInfo.js

  * @return {Array} containing all members of the current struct type
  */
-function getStructMembers (type, stateDefinitions, contractName, location) {


Please add a test for the following type: struct X { uint a; mapping(uint=>uint) b; uint c; }
If such a struct is used in memory, it should be identical to struct X { uint a; uint c; }.

issue https://github.com/ethereum/remix/issues/510

chriseth · 2017-05-22T12:46:42Z

src/solidity/types/Mapping.js

+        type: this.type
+      }
+    }
+    var mapSlot = util.toBN(location.slot).toString(16)


please use helper here

chriseth · 2017-05-22T12:49:48Z

src/storage/mappingPreimages.js

@@ -0,0 +1,58 @@
+var global = require('../helpers/global')


Please document all functions here.

chriseth · 2017-05-22T12:52:44Z

src/storage/storageViewer.js


 class StorageViewer {
  constructor (_context, _storageResolver, _traceManager) {
    this.context = _context
    this.storageResolver = _storageResolver
+    // contains [mappingSlot][mappingkey] = preimage
+    // this map is renewed for each execution step


Should only do it once at the beginning of the debugging session and use the trace otherwise.

chriseth · 2017-05-22T12:54:37Z

src/web3Provider/web3VmProvider.js

+function pushSha3Preimage (self, sha3Input) {
+  var preimage = sha3Input
+  var imageHash = ethutil.sha3('0x' + sha3Input).toString('hex')
+  self.sha3Preimages[imageHash] = {


Using self in that way here looks a bit weird. What about moving the code to the point where this function is called?

chriseth · 2017-05-22T12:56:10Z

src/web3Provider/web3VmProvider.js

+  memoryStart = parseInt(memStartDec) * 2
+  var memLengthDec = (new ethutil.BN(memoryLength.replace('0x', ''), 16).toString(10))
+  memoryLength = parseInt(memLengthDec) * 2
+  var memoryHex = memory.join('')


Please avoid this (high complexity).

- add we3.debug.preimage - move mapping resolver from soliditystate to storageViewer

chriseth · 2017-05-23T11:11:45Z

src/web3Provider/web3VmProvider.js

+  var subMemoryIndex = Math.floor(memoryStart / 32)
+  var sha3Input = ''
+  while (sha3Input.length < memoryLength) {
+    sha3Input += memory[subMemoryIndex]


Ah, I think I misunderstood your question earlier. Unfortunately this won't work. Types in memory most of the time have a length of 32 bytes but

they might be offset by some amount

this does not apply to sha3

yann300 · 2017-05-24T10:15:53Z

src/storage/storageViewer.js

+    *
+    * @param {Function} callback
+    */
+  async mappingsLocation () {


return a promise

chriseth added the in progress label May 2, 2017

yann300 force-pushed the mapping-decoder branch 2 times, most recently from 5a26444 to ca9a5d2 Compare May 18, 2017 14:50

yann300 changed the title ~~[WIP] initial support for mapping types~~ initial support for mapping types May 18, 2017

yann300 force-pushed the mapping-decoder branch from ca9a5d2 to 686741b Compare May 18, 2017 15:06

cdetrio commented May 18, 2017

View reviewed changes

chriseth suggested changes May 22, 2017

View reviewed changes

cdetrio and others added 3 commits May 22, 2017 15:31

initial support for mapping types

8b76861

move sha3 mapping from traceChache to web3VMProvider

aef34c6

- add we3.debug.preimage - move mapping resolver from soliditystate to storageViewer

remove location from getStructMembers

3ff34f1

yann300 force-pushed the mapping-decoder branch from 75cf9f4 to d4310fd Compare May 22, 2017 13:32

use helper

ce16924

yann300 force-pushed the mapping-decoder branch from d4310fd to ce16924 Compare May 22, 2017 13:48

yann300 added 4 commits May 22, 2017 16:00

add doc

0958b7e

remove uneeded function

72e1b87

extract sha3 input

e59e564

cache the preimages for a debug session (using the initial state)

ce3caa6

chriseth reviewed May 23, 2017

View reviewed changes

fix sha3 input extract

075fa4e

yann300 reviewed May 24, 2017

View reviewed changes

src/storage/storageViewer.js

*

* @param {Function} callback

*/

async mappingsLocation () {

Copy link

Collaborator

yann300 May 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return a promise

yann300 added 2 commits May 24, 2017 12:48

return a promise if mappings alrady requested

4013d35

typo + emptyFill if memory not yet filled

cbd30b8

chriseth approved these changes May 24, 2017

View reviewed changes

yann300 merged commit ef68c8a into ethereum:master May 24, 2017

yann300 removed in progress labels May 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial support for mapping types #498

initial support for mapping types #498

cdetrio commented May 2, 2017

yann300 commented May 4, 2017

cdetrio commented May 4, 2017

cdetrio May 18, 2017

cdetrio May 18, 2017

yann300 commented May 18, 2017

chriseth May 22, 2017

yann300 May 22, 2017

chriseth May 22, 2017

chriseth May 22, 2017

chriseth May 22, 2017

chriseth May 22, 2017

chriseth May 22, 2017

chriseth May 23, 2017

yann300 May 24, 2017

initial support for mapping types #498

initial support for mapping types #498

Conversation

cdetrio commented May 2, 2017

yann300 commented May 4, 2017

cdetrio commented May 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yann300 commented May 18, 2017

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