Skip to content
This repository was archived by the owner on Mar 3, 2021. It is now read-only.

initial support for mapping types#498

Merged
yann300 merged 11 commits into
remix-project-org:masterfrom
cdetrio:mapping-decoder
May 24, 2017
Merged

initial support for mapping types#498
yann300 merged 11 commits into
remix-project-org:masterfrom
cdetrio:mapping-decoder

Conversation

@cdetrio
Copy link
Copy Markdown
Contributor

@cdetrio cdetrio commented May 2, 2017

Work in progress, don't merge.

remix mappings - screen shot 2017-05-02 at 12 17 01 pm

@yann300
Copy link
Copy Markdown
Contributor

yann300 commented May 4, 2017

cool!!

@cdetrio
Copy link
Copy Markdown
Contributor Author

cdetrio commented May 4, 2017

@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).

@yann300 yann300 force-pushed the mapping-decoder branch 2 times, most recently from 5a26444 to ca9a5d2 Compare May 18, 2017 14:50
@yann300 yann300 changed the title [WIP] initial support for mapping types initial support for mapping types May 18, 2017
@yann300 yann300 force-pushed the mapping-decoder branch from ca9a5d2 to 686741b Compare May 18, 2017 15:06
Comment thread src/solidity/decodeInfo.js Outdated
* @return {Array} containing all members of the current struct type
*/
function getStructMembers (type, stateDefinitions, contractName) {
function getStructMembers (type, stateDefinitions, contractName, location) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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).

value = value.replace('0x', '').replace(/(..)/g, '%$1')
var ret = {
// length: decoded.length, // unneeded, only dynamicBytes uses length
length: decoded.length,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

@yann300
Copy link
Copy Markdown
Contributor

yann300 commented May 18, 2017

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

* @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, location) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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; }.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread src/solidity/types/Mapping.js Outdated
type: this.type
}
}
var mapSlot = util.toBN(location.slot).toString(16)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please use helper here

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

Choose a reason for hiding this comment

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

Please document all functions here.

Comment thread src/storage/storageViewer.js Outdated
this.context = _context
this.storageResolver = _storageResolver
// contains [mappingSlot][mappingkey] = preimage
// this map is renewed for each execution step
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread src/web3Provider/web3VmProvider.js Outdated
function pushSha3Preimage (self, sha3Input) {
var preimage = sha3Input
var imageHash = ethutil.sha3('0x' + sha3Input).toString('hex')
self.sha3Preimages[imageHash] = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread src/web3Provider/web3VmProvider.js Outdated
memoryStart = parseInt(memStartDec) * 2
var memLengthDec = (new ethutil.BN(memoryLength.replace('0x', ''), 16).toString(10))
memoryLength = parseInt(memLengthDec) * 2
var memoryHex = memory.join('')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please avoid this (high complexity).

cdetrio and others added 3 commits May 22, 2017 15:31
@yann300 yann300 force-pushed the mapping-decoder branch from 75cf9f4 to d4310fd Compare May 22, 2017 13:32
@yann300 yann300 force-pushed the mapping-decoder branch from d4310fd to ce16924 Compare May 22, 2017 13:48
Comment thread src/web3Provider/web3VmProvider.js Outdated
var subMemoryIndex = Math.floor(memoryStart / 32)
var sha3Input = ''
while (sha3Input.length < memoryLength) {
sha3Input += memory[subMemoryIndex]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  1. they might be offset by some amount
  2. this does not apply to sha3

*
* @param {Function} callback
*/
async mappingsLocation () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return a promise

@yann300 yann300 merged commit ef68c8a into remix-project-org:master May 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants