Track callee object creation/update to bill correctly in caller

[aionbot]

Issue created by jeff-aion (on Wednesday Oct 24, 2018 at 20:03 GMT)

As noticed in #268, there are some cases where the reentrant storage logic might cause us to incorrectly bill a new instance at every level of the call stack where it is reachable but we will also do the same to bill for an updating write to an instance at every level.

These are both examples of the same problem, where the callee transaction creates or updates an instance which, when written-back to the caller, causes it to now see a disagreement with its own caller.

Note that we also need to be sure that the new instance logic can handle this case: if the callee creates a new instance but the caller orphans it, will we write it to storage? We MUST make sure that this happens if the commit went through since the contract was billed for the new instance but it will never be able to GC it unless the storage can account for its creation.

I suspect that, in both cases, we can solve this with a map of INode to Extent, created on every commit and passed back to the caller as "what they believe they wrote" (WARNING: INode references are per-transaction, so the same instance in callee and caller might not match). At each level of the stack, the check if this instance was updated should first consult this map, before going to the caller or disk storage.

Additionally, these probably need to be "inherited" back into each caller frame so that even an orphaned write or orphaned new instance will still be forced to write-back if the call stack commits all the way back to the disk.

Note that this should also supersede the work done to fix the "islands of change" problem (#249) since that is really just a special-case of this problem.

[aionbot]

Comment by jeff-aion (on Monday Nov 19, 2018 at 14:41 GMT)

We want this early in the Beta timeframe (it is a complex bug) but it doesn't technically gate the Beta release.

[aionbot]

Comment by jeff-aion (on Wednesday Nov 28, 2018 at 18:12 GMT)

Note that the above description of using INode as the key for this structure is incorrect as those only exist directly at the disk level. Instead, IPersistenceToken is probably more appropriate as it is the more general object associated with each instance.

I actually suspect that many of the other maps and lists, tracking reentrant object relationships in ReentrantGraphProcessor, may actually be replaced by the value type in the mapping required to address this issue.

[jeff-aion]

Due to some clarifications around how we want to handle the object graph and storage, in general, this bug won't be fixed. Instead, we will be re-implementing the object graph as a much simpler "read/write everything" design, which is tightly constrained in terms of size (using explicit key-value store for long-lived data for which membership proofs are required). Internally, this is tracked as AKI-85.