Fix various decoding errors with reference types #1677

haltman-at · 2019-01-29T19:32:14Z

This PR fixes a number of decoding errors with reference types. Specifically:

Storage arrays whose base type is longer than a word now work properly.
Mappings whose value type is longer than a word now work properly (if they didn't already?).
Mappings whose value type is a signed integer shorter than a full word now work properly.
Statically-sized arrays in memory now work properly.
Struct types with mappings in them now work properly in memory. Actually, now structs in memory work at all! Turns out there was a bug causing errors when you attempted to decode any struct in memory.
Decoding of storage local variables is now supported.

Also, the file truffle-decode-utils/allocation.ts has now been deleted; its contents have been moved into truffle-decoder/allocate/storage.ts, except for normalizeSlot, which is gone. (Turns out it was already unused, and I could have deleted it in my last PR like I wanted to, but I didn't notice that then.)

Note that in order to add support for local variables, I ended up rewriting decode/index.ts to change how the dispatching works there, and added a new file decode/stack.ts, along with some new entry points to storage and memory decoding (decodeStorage, decodeMemory, decodeStorageByAddress).

I also added a test for these fixes; it doesn't fit in well with the other decoding tests, but I figured it was better than not including it. Also incidentally I turned down the timeout on the variable ID tests from 30 seconds (!) to just 4 seconds. Hopefully that's not too short and this PR passes Travis without me having to turn that back up!

…or mappings in memory structs, remove the now-unused baseDefinition().

…storage.ts, adjusting references appropriately. (Also, fix an incorrect invocation of storageSize in mapping decoding.)

…ndex.ts so that it dispatches purely based on the pointer type (adding a new stack.ts for the stack), and add extra dispatcher functions in memory.ts and storage.ts to make this work.

…coding.

…own the timeout on the ID tests from 30 seconds (!) to 4.

gnidan

Here's an initial round of comments while we're waiting on Travis. they're mostly devoid of usefulness.

gnidan · 2019-01-29T19:36:36Z

packages/truffle-decoder/lib/allocate/storage.ts

+};
+
+export interface Slot {
+  key?: any; // TODO:


what's this TODO?

That's preexisting -- I assume @seesemichaelj put that there. I assume it's about the fact that key is of type any, which should be narrowed down to something more specific (I figure I'll get to that when I get to going over mapping keys more generally). You're seeing it anew due to it having moved to a different file.

Or maybe I'll wait until the new decoder output format, actually. In any case the point is I'll handle it eventually.

gnidan · 2019-01-29T19:38:41Z

packages/truffle-decoder/lib/decode/storage.ts

 import BN from "bn.js";
 import Web3 from "web3";
 import { EvmStruct, EvmMapping } from "../interface/contract-decoder";
 import clonedeep from "lodash.clonedeep";

-export default async function decodeStorageReference(definition: DecodeUtils.AstDefinition, pointer: StoragePointer, info: EvmInfo, web3?: Web3, contractAddress?: string): Promise<any> {
+export default async function decodeStorage(definition: DecodeUtils.AstDefinition, pointer: StoragePointer, info: EvmInfo, web3?: Web3, contractAddress?: string): Promise <any> {


Gosh this line is long

Then turn on prettier :P

packages/truffle-decoder/lib/decode/storage.ts

gnidan · 2019-01-29T19:49:03Z

packages/truffle-decoder/lib/decode/storage.ts

@@ -269,7 +351,7 @@ export default async function decodeStorageReference(definition: DecodeUtils.Ast
          }

          result.members[memberName] =
-            await decode(definition.typeName.valueType, valuePointer, info, web3, contractAddress);
+            await decode(valueDefinition, valuePointer, info, web3, contractAddress);
        }
      }


gosh, changes to this file are sure getting difficult to read about. it all seems reasonable though. we should think about getting this more organized so it doesn't require as large a brain-cache size to look at PRs.

coveralls · 2019-01-29T20:32:42Z

Coverage decreased (-33.5%) to 36.325% when pulling 63ca4b5 on reference-types into 28815e6 on develop.

… typo and remove a needless empty line.

…. Also, fix an error in decodeStorageByAddress where it didn't pass enough arguments to read.

haltman-at · 2019-01-30T01:35:39Z

Let me add a comment on this most recent commit, because it changes the structure somewhat. Literal pointers are now no longer passed to decodeValue, since they might not be values (the old code got this right, silly me), but rather to decodeLiteral. Since literal pointers come from the stack (unless they're string literals), decodeLiteral resides in stack.ts, with decodeStack now calling it. String literals will be dispatched by decodeLiteral to decodeValue, but this is kind of a hack, and I'm intending to change this in a later PR.

gnidan

Exciting effort! Looks good @haltman-at

haltman-at added 5 commits January 28, 2019 23:16

Fix storage arrays, fix mappings, fix static memory arrays, account f…

aaad5f0

…or mappings in memory structs, remove the now-unused baseDefinition().

Delete normalizeSlot, and move the rest of allocation.ts to allocate/…

a22f1b6

…storage.ts, adjusting references appropriately. (Also, fix an incorrect invocation of storageSize in mapping decoding.)

Add support for local storage variables. In the process, reorganize i…

47f2006

…ndex.ts so that it dispatches purely based on the pointer type (adding a new stack.ts for the stack), and add extra dispatcher functions in memory.ts and storage.ts to make this work.

Fix various compile errors. Also, fix some errors in memory struct de…

c805b7d

…coding.

Add a test to test the decoding issues fixed in this PR. Also, turn d…

7c49b89

…own the timeout on the ID tests from 30 seconds (!) to 4.

haltman-at requested a review from gnidan January 29, 2019 19:32

Turn up the timeout. Sigh.

04c3645

gnidan reviewed Jan 29, 2019

View reviewed changes

haltman-at added 2 commits January 29, 2019 18:32

Remove baseDefinition, since it's no longer used. Also, fix a comment…

fed58ed

… typo and remove a needless empty line.

Add decodeLiteral, because literal pointers aren't always just values…

d8b52a4

…. Also, fix an error in decodeStorageByAddress where it didn't pass enough arguments to read.

Add HACK comment. :P

63ca4b5

gnidan approved these changes Jan 30, 2019

View reviewed changes

haltman-at merged commit 67ac461 into develop Jan 30, 2019

haltman-at deleted the reference-types branch January 30, 2019 02:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix various decoding errors with reference types #1677

Fix various decoding errors with reference types #1677

haltman-at commented Jan 29, 2019 •

edited

gnidan left a comment

gnidan Jan 29, 2019

haltman-at Jan 29, 2019 •

edited

haltman-at Jan 29, 2019

gnidan Jan 29, 2019

haltman-at Jan 29, 2019

gnidan Jan 29, 2019

coveralls commented Jan 29, 2019 •

edited

haltman-at commented Jan 30, 2019

gnidan left a comment

Fix various decoding errors with reference types #1677

Fix various decoding errors with reference types #1677

Conversation

haltman-at commented Jan 29, 2019 • edited

gnidan left a comment

Choose a reason for hiding this comment

gnidan Jan 29, 2019

Choose a reason for hiding this comment

haltman-at Jan 29, 2019 • edited

Choose a reason for hiding this comment

haltman-at Jan 29, 2019

Choose a reason for hiding this comment

gnidan Jan 29, 2019

Choose a reason for hiding this comment

haltman-at Jan 29, 2019

Choose a reason for hiding this comment

gnidan Jan 29, 2019

Choose a reason for hiding this comment

coveralls commented Jan 29, 2019 • edited

haltman-at commented Jan 30, 2019

gnidan left a comment

Choose a reason for hiding this comment

haltman-at commented Jan 29, 2019 •

edited

haltman-at Jan 29, 2019 •

edited

coveralls commented Jan 29, 2019 •

edited