Disallow unreferenced subcontainers and subcontainers referenced by b…

…oth EOFCREATE and RETURNCONTRACT (#916)

- Disallow unreferenced subcontainers, following the spec change
ipsilon/eof#122.
- Disallow EOFCREATE and RETURNCONTRACT referencing the same container,
following the spec change ipsilon/eof#135.

circle.yml

@@ -407,8 +407,11 @@ jobs:
  - run:
  name: "EOF pre-release execution spec tests (state_tests)"
  working_directory: ~/build
+ # TODO: Disabled tests:
+ # prague/eip7692_eof_v1/eip7620_eof_create/eofcreate.eofcreate_in_initcode_reverts: contains invalid EOF container: unreferenced subcontainer
  command: >
  bin/evmone-statetest ~/spec-tests/fixtures/state_tests
+ --gtest_filter=-prague/eip7692_eof_v1/eip7620_eof_create/eofcreate.eofcreate_in_initcode_reverts
  - run:
  name: "EOF pre-release execution spec tests (blockchain_tests)"
  working_directory: ~/build
@@ -417,8 +420,12 @@ jobs:
  - run:
  name: "EOF pre-release execution spec tests (eof_tests)"
  working_directory: ~/build
+ # TODO: Disabled tests:
+ # prague/eip7692_eof_v1/eip3540_eof_v1/all_opcodes_in_container.all_opcodes_in_container
+ # prague/eip7692_eof_v1/eip3540_eof_v1/code_validation.legacy_initcode_valid_eof_v1_contract: unreferenced subcontainers, fixed in https://github.com/ethereum/execution-spec-tests/pull/649
  command: >
- bin/evmone-eoftest ~/spec-tests/fixtures/eof_tests --gtest_filter=-prague/eip7692_eof_v1/eip3540_eof_v1/all_opcodes_in_container.all_opcodes_in_container
+ bin/evmone-eoftest ~/spec-tests/fixtures/eof_tests 
+ --gtest_filter=-prague/eip7692_eof_v1/eip3540_eof_v1/all_opcodes_in_container.all_opcodes_in_container:prague/eip7692_eof_v1/eip3540_eof_v1/code_validation.legacy_initcode_valid_eof_v1_contract
  - collect_coverage_gcc
  - upload_coverage:
  flags: eof_execution_spec_tests
@@ -471,8 +478,11 @@ jobs:
  - run:
  name: "EOF validation tests"
  working_directory: ~/build
- command: |
+ # TODO: Disabled tests:
+ # efValidation.EOF1_embedded_container_:efValidation.EOF1_eofcreate_valid_:efValidation.EOF1_section_order_: unreferenced subcontainer
+ command: >
  bin/evmone-eoftest ~/tests/EOFTests
+ --gtest_filter=-:efValidation.EOF1_embedded_container_:efValidation.EOF1_eofcreate_valid_:efValidation.EOF1_section_order_
  - collect_coverage_gcc
  - upload_coverage:
  flags: ethereum_tests

lib/evmone/eof.cpp

@@ -329,7 +329,7 @@ std::variant<InstructionValidationResult, EOFValidationError> validate_instructi
 
  if (op == OP_RETURNCONTRACT)
  {
- if (kind == ContainerKind::runtime || kind == ContainerKind::initcode_runtime)
+ if (kind == ContainerKind::runtime)
  return EOFValidationError::incompatible_container_kind;
  }
 
@@ -338,7 +338,7 @@ std::variant<InstructionValidationResult, EOFValidationError> validate_instructi
  }
  else if (op == OP_RETURN || op == OP_STOP)
  {
- if (kind == ContainerKind::initcode || kind == ContainerKind::initcode_runtime)
+ if (kind == ContainerKind::initcode)
  return EOFValidationError::incompatible_container_kind;
  }
  else
@@ -683,8 +683,7 @@ EOFValidationError validate_eof1(
  {
  if (main_container == container)
  return EOFValidationError::toplevel_container_truncated;
- if (container_kind == ContainerKind::initcode ||
- container_kind == ContainerKind::initcode_runtime)
+ if (container_kind == ContainerKind::initcode)
  return EOFValidationError::eofcreate_with_truncated_container;
  }
 
@@ -696,13 +695,14 @@ EOFValidationError validate_eof1(
  const bool eofcreate = subcontainer_referenced_by_eofcreate[subcont_idx];
  const bool returncontract = subcontainer_referenced_by_returncontract[subcont_idx];
 
- // TODO Validate whether subcontainer was referenced by any instruction
+ if (eofcreate && returncontract)
+ return EOFValidationError::ambiguous_container_kind;
+ if (!eofcreate && !returncontract)
+ return EOFValidationError::unreferenced_subcontainer;
 
- auto subcontainer_kind = ContainerKind::initcode_runtime;
- if (!eofcreate)
- subcontainer_kind = ContainerKind::runtime;
- else if (!returncontract)
- subcontainer_kind = ContainerKind::initcode;
+ const auto subcontainer_kind =
+ (eofcreate ? ContainerKind::initcode : ContainerKind::runtime);
+ assert(subcontainer_kind == ContainerKind::initcode || returncontract);
 
  container_queue.push({subcontainer, subcontainer_kind});
  }
@@ -988,6 +988,8 @@ std::string_view get_error_message(EOFValidationError err) noexcept
  return "incompatible_container_kind";
  case EOFValidationError::container_size_above_limit:
  return "container_size_above_limit";
+ case EOFValidationError::unreferenced_subcontainer:
+ return "unreferenced_subcontainer";
  case EOFValidationError::impossible:
  return "impossible";
  }

lib/evmone/eof.hpp

@@ -142,6 +142,7 @@ enum class EOFValidationError
  ambiguous_container_kind,
  incompatible_container_kind,
  container_size_above_limit,
+ unreferenced_subcontainer,
 
  impossible,
 };
@@ -152,9 +153,6 @@ enum class ContainerKind : uint8_t
  initcode,
  /// Container that uses STOP/RETURN. Can be returned by RETURNCONTRACT.
  runtime,
- /// Container that uses only REVERT/INVALID or does not terminate execution.
- /// Can be used in any context.
- initcode_runtime,
 };
 
 /// Determines the EOF version of the container by inspecting container's EOF prefix.

test/eoftest/eoftest_runner.cpp

@@ -40,8 +40,6 @@ ContainerKind container_kind_from_string(std::string_view s)
  return ContainerKind::runtime;
  else if (s == "initcode")
  return ContainerKind::initcode;
- else if (s == "initcode_runtime")
- return ContainerKind::initcode_runtime;
  else
  throw std::invalid_argument{"unknown container kind"};
 }

test/unittests/eof_test.cpp

@@ -42,6 +42,12 @@ TEST(eof, read_valid_eof1_header)
  code_sections_256 += "E5" + hex(big_endian(static_cast<uint16_t>(i + 1)));
  code_sections_256 += "5B5B00";
 
+ std::string eofcreate_256;
+ for (int i = 0; i < 256; ++i)
+ eofcreate_256 += "5F5F5F5FEC" + hex(static_cast<uint8_t>(i)) + "50";
+ eofcreate_256 += "00";
+ const auto eofcreate_256_size = static_cast<uint16_t>(eofcreate_256.size() / 2);
+
  const TestCase test_cases[] = {
  {"EF00 01 010004 0200010001 040000 00 00800000 00", 4, 0, {1}, {}},
  {"EF00 01 010004 0200010006 040000 00 00800002 600160005500", 4, 0, {6}, {}},
@@ -59,26 +65,29 @@ TEST(eof, read_valid_eof1_header)
  {"EF00 01 010400 020100" + hex(256 * bytecode("0003")) + " 041000 00 " +
  hex(256 * bytecode("00800000")) + code_sections_256 + std::string(8192, 'F'),
  4 * 256, 4096, std::vector<uint16_t>(256, 3), {}},
- {"EF00 01 010004 0200010001 0300010014 040000 00 00800000 00 "
+ {"EF00 01 010004 0200010007 0300010014 040000 00 00800004 5F5F5F5FEC0000 "
  "EF000101000402000100010400000000800000FE",
- 4, 0, {1}, {20}},
- {"EF00 01 010004 0200010001 030003001400160018 040000 00 00800000 00 "
+ 4, 0, {7}, {20}},
+ {"EF00 01 010004 0200010015 030003001400160018 040000 00 00800004 "
+ "5F5F5F5FEC00505F5F5F5FEC01505F5F5F5FEC0200 "
  "EF000101000402000100010400000000800000FE EF0001010004020001000304000000008000025F5FFD "
- "EF00010100040200010005040000000080000260015F5500",
- 4, 0, {1}, {20, 22, 24}},
- {"EF00 01 010004 0200010001 030003001400160018 040003 00 00800000 00 "
+ "EF00010100040200010005040000000080000260015F55FE",
+ 4, 0, {21}, {20, 22, 24}},
+ {"EF00 01 010004 0200010015 030003001400160018 040003 00 00800004 "
+ "5F5F5F5FEC00505F5F5F5FEC01505F5F5F5FEC0200 "
  "EF000101000402000100010400000000800000FE EF0001010004020001000304000000008000025F5FFD "
- "EF00010100040200010005040000000080000260015F5500 ddeeff",
- 4, 3, {1}, {20, 22, 24}},
- {"EF00 01 01000C 020003000300030001 030003001400160018 040003 00 008000000080000000800000 "
- "E50001 E50002 00 EF000101000402000100010400000000800000FE "
+ "EF00010100040200010005040000000080000260015F55FE ddeeff",
+ 4, 3, {21}, {20, 22, 24}},
+ {"EF00 01 01000C 020003000300030015 030003001400160018 040003 00 008000000080000000800004 "
+ "E50001 E50002 5F5F5F5FEC00505F5F5F5FEC01505F5F5F5FEC0200 "
+ "EF000101000402000100010400000000800000FE "
  "EF0001010004020001000304000000008000025F5FFD "
- "EF00010100040200010005040000000080000260015F5500 ddeeff",
- 12, 3, {3, 3, 1}, {20, 22, 24}},
- {"EF00 01 010004 0200010001 030100" + hex(256 * bytecode("0014")) +
- "040000 00 00800000 00" +
+ "EF00010100040200010005040000000080000260015F55FE ddeeff",
+ 12, 3, {3, 3, 21}, {20, 22, 24}},
+ {"EF00 01 010004 020001" + hex(big_endian(eofcreate_256_size)) + "030100" +
+ hex(256 * bytecode("0014")) + "040000 00 00800004" + eofcreate_256 +
  hex(256 * bytecode("EF000101000402000100010400000000800000FE")),
- 4, 0, {1}, std::vector<uint16_t>(256, 20)},
+ 4, 0, {eofcreate_256_size}, std::vector<uint16_t>(256, 20)},
  };
 
  for (const auto& test_case : test_cases)

test/unittests/eof_validation.cpp

@@ -95,6 +95,8 @@ std::string_view get_tests_error_message(EOFValidationError err) noexcept
  return "EOF_IncompatibleContainerKind";
  case EOFValidationError::container_size_above_limit:
  return "EOF_ContainerSizeAboveLimit";
+ case EOFValidationError::unreferenced_subcontainer:
+ return "EOF_UnreferencedSubcontainer";
  case EOFValidationError::impossible:
  return "impossible";
  }
@@ -109,8 +111,6 @@ std::string_view to_string(ContainerKind container_kind) noexcept
  return "runtime";
  case (ContainerKind::initcode):
  return "initcode";
- case (ContainerKind::initcode_runtime):
- return "initcode_runtime";
  }
  return "<unknown>";
 }