YAS 277 Logarithmic-time JUMP transpilation

[ben-chain]

Description

Introduces a binary search tree in place of the switch case Jump table logic to reduce gas overhead. WIP as it needs some comments and other structural shifts, but core functionality is there and all integrations seem to pass.

Questions

• I deleted a good bit of testing done in the function validateJumpBytecode simply because it was hard to reproduce that functionality with this new structure. Hopefully rigorous integration tests (turns out the original jump-integration.spec.ts was missing awaits and completely useless) make up for this, but if someone can think of a way to duplicate what validateJumpBytecode was doing, happy to add back. @willmeister maybe you're best to give a recommendation there :)

Metadata

Fixes

• Fixes YAS 277

Contributing Agreement

• I have read and understood the Optimism Contributing Guide and Code of Conduct and am following those guidelines in this pull request.

[willmeister]

Looks like a good first pass!

Left a few comments to improve the performance and readability. It sounds like this works, but I'm concerned about the tech debt of reinventing the wheel. If we find a bug in any of this, we will need to take a while to familiarize ourselves with the code before being able to debug it.

For instance, if using a standard BST, even someone not familiar with the code but familiar with data structures can print it, easily tell if something is wrong, and know immediately where to fix it. With the tree created here, we first need to figure out how it works in order to tell if it has actually been assembled correctly, and then we need to read through the non-standard code assembling it to figure out where we might be able to fix it.

[willmeister]

This will return without error if the conditions documented here are not met. Bare minimum, should throw if assumptions aren't met, even better, automatically fix it, doing the sorting here instead of in every caller.

[willmeister]

Should check to make sure keys.length === values.length. An easy way to do that is to just declare the input type:

export interface NumericMap {
  [key: number]: number
}

That changes your map to

    Object.keys(input)
      .sort((a, b) => a < b ? -1 : a === b ? 0 : 1 )
      .map((key, i) => {
        return {
          key,
          value: input[i],
          nodeId: i,
        }
      })

[ben-chain]

Leaving without this check for now as we do it in jump-replacement.ts already, if you think that sounds reasonable

[willmeister]

Looks good!

Would pull out two functions (listed in comments) to make it read a bit more clearly, but this is much better!

[willmeister]

Would create a helper function for this. It'll make the logic read more clearly:

if (isRightNode) {
    bytecodeToReturn.push(generateBinarySearchTreeNodeJumpdest(node))
}

bytecodeToReturn.push(generateNodeEqualityCheckBytecode(...))

if (!node.left && !node.right) {
  // stop
}

bytecodeToReturn.push(generateCheckIfGreaterAndJumpToRightChildBytecode(...))

bytecodeToReturn.push(
   ...getBytecodeForSubtreeRoot(node.left, false),
   ...getBytecodeForSubtreeRoot(node.right, true)
)

return bytecodeToReturn

[willmeister]

Would create a helper function for this. It'll make the logic read more clearly:

if (isRightNode) {
    bytecodeToReturn.push(generateBinarySearchTreeNodeJumpdest(node))
}

bytecodeToReturn.push(generateNodeEqualityCheckBytecode(...))

if (!node.left && !node.right) {
  // stop
}

bytecodeToReturn.push(generateCheckIfGreaterAndJumpToRightChildBytecode(...))

bytecodeToReturn.push(
   ...getBytecodeForSubtreeRoot(node.left, false),
   ...getBytecodeForSubtreeRoot(node.right, true)
)

return bytecodeToReturn

[willmeister]

Maybe rename to getJumpFoundSuccessfullyJumpdestIndexBytecode now that it's not a switch and the algo is not relevant?