From 4e1e2a54319d6c3970e3463976fb3f513e397951 Mon Sep 17 00:00:00 2001 From: jonesho <81145364+jonesho@users.noreply.github.com> Date: Thu, 17 Oct 2024 23:34:21 +0800 Subject: [PATCH 1/6] =?UTF-8?q?feat:=20rewrite=20error=20logs=20from=20blo?= =?UTF-8?q?b=20submission=20eth=5Fcall=20due=20to=20insuffi=E2=80=A6=20(#1?= =?UTF-8?q?57)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: rewrite error logs from blob submission eth_call due to insufficient gas prices * feat: revised unit test titles * feat: revised the log filter regex for specifically insufficient gas price during blob submission eth_call * feat: reverted back to INFO level in log4j2-dev.xml * feat: removed log rewriter known error for insufficient gas fee and update rewrite wordings * feat: use variable to hold regex objects and change Error in test to RuntimeError --- .../ethereum/submission/LoggingHelper.kt | 47 +++++- .../ethereum/submission/LoggingHelperTest.kt | 144 ++++++++++++++++++ 2 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 coordinator/ethereum/blob-submitter/src/test/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelperTest.kt diff --git a/coordinator/ethereum/blob-submitter/src/main/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelper.kt b/coordinator/ethereum/blob-submitter/src/main/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelper.kt index d68e6059f..814fc33b0 100644 --- a/coordinator/ethereum/blob-submitter/src/main/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelper.kt +++ b/coordinator/ethereum/blob-submitter/src/main/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelper.kt @@ -2,6 +2,20 @@ package net.consensys.zkevm.ethereum.submission import org.apache.logging.log4j.Logger +private const val insufficientGasFeeRegexStr = + "(max fee per (blob )?gas less than block (blob gas|base) fee:" + + " address 0x[a-fA-F0-9]{40},? (blobGasFeeCap|maxFeePerGas): [0-9]+," + + " (blobBaseFee|baseFee): [0-9]+ \\(supplied gas [0-9]+\\))" + +private val insufficientGasFeeRegex = Regex(insufficientGasFeeRegexStr) +private val maxFeePerBlobGasRegex = Regex("max fee per blob gas|blobGasFeeCap") + +private fun rewriteInsufficientGasFeeErrorMessage(errorMessage: String): String? { + return insufficientGasFeeRegex.find(errorMessage)?.groupValues?.first() + ?.replace("max fee per gas", "maxFeePerGas") + ?.replace(maxFeePerBlobGasRegex, "maxFeePerBlobGas") +} + fun logUnhandledError( log: Logger, errorOrigin: String, @@ -30,13 +44,32 @@ fun logSubmissionError( error: Throwable, isEthCall: Boolean = false ) { + var matchedInsufficientGasFeeRegex = false val ethMethod = if (isEthCall) "eth_call" else "eth_sendRawTransaction" + val errorMessage = if (isEthCall) { + error.message?.let { + rewriteInsufficientGasFeeErrorMessage(it)?.also { + matchedInsufficientGasFeeRegex = true + } + } ?: error.message + } else { + error.message + } - log.error( - logMessage, - ethMethod, - intervalString, - error.message, - error - ) + if (matchedInsufficientGasFeeRegex) { + log.info( + logMessage, + ethMethod, + intervalString, + errorMessage + ) + } else { + log.error( + logMessage, + ethMethod, + intervalString, + errorMessage, + error + ) + } } diff --git a/coordinator/ethereum/blob-submitter/src/test/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelperTest.kt b/coordinator/ethereum/blob-submitter/src/test/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelperTest.kt new file mode 100644 index 000000000..b288799ab --- /dev/null +++ b/coordinator/ethereum/blob-submitter/src/test/kotlin/net/consensys/zkevm/ethereum/submission/LoggingHelperTest.kt @@ -0,0 +1,144 @@ +package net.consensys.zkevm.ethereum.submission + +import org.apache.logging.log4j.Logger +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.mockito.Mockito.mock +import org.mockito.kotlin.eq +import org.mockito.kotlin.verify + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class LoggingHelperTest { + private lateinit var logger: Logger + + private val blobSubmissionFailedLogMsg = "{} for blob submission failed: blob={} errorMessage={}" + private val blobSubmissionFailedIntervalStr = "[4025752..4031499]5748" + private val insufficientMaxFeePerBlobGasErrMsg = "org.web3j.tx.exceptions.ContractCallException:" + + " Contract Call has been reverted by the EVM with the reason:" + + " 'err: max fee per blob gas less than block blob gas fee:" + + " address 0x47C63d1E391FcB3dCdC40C4d7fA58ADb172f8c38 blobGasFeeCap: 1875810596," + + " blobBaseFee: 1962046498 (supplied gas 1000000)'. revertReason=UNKNOWN errorData=null" + + private val insufficientMaxFeePerGasErrMsg = "org.web3j.tx.exceptions.ContractCallException:" + + " Contract Call has been reverted by the EVM with the reason:" + + " 'err: max fee per gas less than block base fee:" + + " address 0x47C63d1E391FcB3dCdC40C4d7fA58ADb172f8c38, maxFeePerGas: 300000000000," + + " baseFee: 302246075616 (supplied gas 1000000)'. revertReason=UNKNOWN errorData=null" + + private val unknownErrMsg = "org.web3j.tx.exceptions.ContractCallException:" + + " Contract Call has been reverted by the EVM with the reason:" + + " 'err: unknown error:" + + " address 0x47C63d1E391FcB3dCdC40C4d7fA58ADb172f8c38'. revertReason=UNKNOWN errorData=null" + + @BeforeEach + fun setUp() { + logger = mock() + } + + @Test + fun `insufficient max fee per gas with isEthCall is true triggers rewrite info message`() { + val error = RuntimeException(insufficientMaxFeePerGasErrMsg) + logSubmissionError( + log = logger, + logMessage = blobSubmissionFailedLogMsg, + intervalString = blobSubmissionFailedIntervalStr, + error = error, + isEthCall = true + ) + val expectedErrorMessage = + "maxFeePerGas less than block base fee:" + + " address 0x47C63d1E391FcB3dCdC40C4d7fA58ADb172f8c38, maxFeePerGas: 300000000000," + + " baseFee: 302246075616 (supplied gas 1000000)" + + verify(logger).info( + eq(blobSubmissionFailedLogMsg), + eq("eth_call"), + eq(blobSubmissionFailedIntervalStr), + eq(expectedErrorMessage) + ) + } + + @Test + fun `insufficient max fee per blob gas with isEthCall is true triggers rewrite info message`() { + val error = RuntimeException(insufficientMaxFeePerBlobGasErrMsg) + logSubmissionError( + log = logger, + logMessage = blobSubmissionFailedLogMsg, + intervalString = blobSubmissionFailedIntervalStr, + error = error, + isEthCall = true + ) + val expectedErrorMessage = + "maxFeePerBlobGas less than block blob gas fee:" + + " address 0x47C63d1E391FcB3dCdC40C4d7fA58ADb172f8c38 maxFeePerBlobGas: 1875810596," + + " blobBaseFee: 1962046498 (supplied gas 1000000)" + + verify(logger).info( + eq(blobSubmissionFailedLogMsg), + eq("eth_call"), + eq(blobSubmissionFailedIntervalStr), + eq(expectedErrorMessage) + ) + } + + @Test + fun `insufficient max fee per gas with isEthCall is false do not trigger rewrite error message`() { + val error = RuntimeException(insufficientMaxFeePerGasErrMsg) + logSubmissionError( + log = logger, + logMessage = blobSubmissionFailedLogMsg, + intervalString = blobSubmissionFailedIntervalStr, + error = error, + isEthCall = false + ) + + verify(logger).error( + eq(blobSubmissionFailedLogMsg), + eq("eth_sendRawTransaction"), + eq(blobSubmissionFailedIntervalStr), + eq(insufficientMaxFeePerGasErrMsg), + eq(error) + ) + } + + @Test + fun `insufficient max fee per blob gas with isEthCall is false do not trigger rewrite error message`() { + val error = RuntimeException(insufficientMaxFeePerBlobGasErrMsg) + logSubmissionError( + log = logger, + logMessage = blobSubmissionFailedLogMsg, + intervalString = blobSubmissionFailedIntervalStr, + error = error, + isEthCall = false + ) + + verify(logger).error( + eq(blobSubmissionFailedLogMsg), + eq("eth_sendRawTransaction"), + eq(blobSubmissionFailedIntervalStr), + eq(insufficientMaxFeePerBlobGasErrMsg), + eq(error) + ) + } + + @Test + fun `Other error with isEthCall is true do not trigger rewrite error message`() { + val error = RuntimeException(unknownErrMsg) + logSubmissionError( + log = logger, + logMessage = blobSubmissionFailedLogMsg, + intervalString = blobSubmissionFailedIntervalStr, + error = error, + isEthCall = true + ) + + verify(logger).error( + eq(blobSubmissionFailedLogMsg), + eq("eth_call"), + eq(blobSubmissionFailedIntervalStr), + eq(unknownErrMsg), + eq(error) + ) + } +} From 74529e67d89941ac044b375ece7813fa9e6ad79d Mon Sep 17 00:00:00 2001 From: Pedro Novais <1478752+jpnovais@users.noreply.github.com> Date: Thu, 17 Oct 2024 17:23:23 +0100 Subject: [PATCH 2/6] Jackson Serialiser and Deserialisers (#194) * adds jackson Serilaiser and Deserialisers * improve jackson Bytes SerDe --- .../ProofToFinalizeJsonResponseTest.kt | 2 +- gradle/libs.versions.toml | 3 +- .../serialization/jackson/build.gradle | 13 ++ .../build/linea/s11n/jackson/BytesHexSerDe.kt | 51 ++++++ .../linea/s11n/jackson/InstantISO8601SerDe.kt | 22 +++ .../linea/s11n/jackson/NumbersHexSerDe.kt | 47 +++++ .../build/linea/s11n/jackson/ObjectMappers.kt | 36 ++++ .../linea/s11n/jackson/BytesSerDeTest.kt | 160 ++++++++++++++++++ .../s11n/jackson/InstantISO8601SerDeTest.kt | 65 +++++++ .../linea/s11n/jackson/NumbersSerDeTest.kt | 80 +++++++++ settings.gradle | 1 + 11 files changed, 478 insertions(+), 2 deletions(-) create mode 100644 jvm-libs/generic/serialization/jackson/build.gradle create mode 100644 jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/BytesHexSerDe.kt create mode 100644 jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/InstantISO8601SerDe.kt create mode 100644 jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/NumbersHexSerDe.kt create mode 100644 jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/ObjectMappers.kt create mode 100644 jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/BytesSerDeTest.kt create mode 100644 jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/InstantISO8601SerDeTest.kt create mode 100644 jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/NumbersSerDeTest.kt diff --git a/coordinator/clients/prover-client/serialization/src/test/kotlin/net/consensys/zkevm/coordinator/clients/prover/serialization/ProofToFinalizeJsonResponseTest.kt b/coordinator/clients/prover-client/serialization/src/test/kotlin/net/consensys/zkevm/coordinator/clients/prover/serialization/ProofToFinalizeJsonResponseTest.kt index f986fa147..cacdc7602 100644 --- a/coordinator/clients/prover-client/serialization/src/test/kotlin/net/consensys/zkevm/coordinator/clients/prover/serialization/ProofToFinalizeJsonResponseTest.kt +++ b/coordinator/clients/prover-client/serialization/src/test/kotlin/net/consensys/zkevm/coordinator/clients/prover/serialization/ProofToFinalizeJsonResponseTest.kt @@ -60,7 +60,7 @@ class ProofToFinalizeJsonResponseTest { val jsonParser = jsonNode.traverse() while (!jsonParser.isClosed) { if (jsonParser.nextToken() == JsonToken.FIELD_NAME) { - keys.add(jsonParser.currentName) + keys.add(jsonParser.currentName()) } } return keys diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 9a5be47e1..725044e1a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -8,7 +8,7 @@ jreleaser = {id = "org.jreleaser", version = "1.13.1"} besu = "22.4.2" caffeine = "3.1.6" hoplite = "2.7.5" -jackson = "2.14.2" +jackson = "2.18.0" jna = "5.14.0" junit = "5.10.1" kotlinxDatetime = "0.4.0" @@ -23,5 +23,6 @@ tuweni = "2.3.1" vertx = "4.5.0" web3j = "4.12.0" wiremock = "3.0.1" +jsonUnit = "3.4.1" blobCompressor = "0.0.4" blobShnarfCalculator = "0.0.4" diff --git a/jvm-libs/generic/serialization/jackson/build.gradle b/jvm-libs/generic/serialization/jackson/build.gradle new file mode 100644 index 000000000..6c14517dc --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/build.gradle @@ -0,0 +1,13 @@ +plugins { + id 'net.consensys.zkevm.kotlin-library-conventions' +} + +dependencies { + implementation(project(':jvm-libs:kotlin-extensions')) + api "com.fasterxml.jackson.core:jackson-annotations:${libs.versions.jackson.get()}" + api "com.fasterxml.jackson.core:jackson-databind:${libs.versions.jackson.get()}" + api "com.fasterxml.jackson.module:jackson-module-kotlin:${libs.versions.jackson.get()}" + api "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${libs.versions.jackson.get()}" + + testImplementation "net.javacrumbs.json-unit:json-unit-assertj:${libs.versions.jsonUnit.get()}" +} diff --git a/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/BytesHexSerDe.kt b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/BytesHexSerDe.kt new file mode 100644 index 000000000..e3e6874d6 --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/BytesHexSerDe.kt @@ -0,0 +1,51 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.core.JsonGenerator +import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.JsonDeserializer +import com.fasterxml.jackson.databind.JsonSerializer +import com.fasterxml.jackson.databind.SerializerProvider +import java.util.HexFormat + +private val hexFormatter = HexFormat.of() + +object ByteArrayToHexSerializer : JsonSerializer() { + override fun serialize(value: ByteArray, gen: JsonGenerator, serializers: SerializerProvider?) { + gen.writeString("0x" + hexFormatter.formatHex(value)) + } + + override fun handledType(): Class { + return ByteArray::class.java + } +} + +object ByteToHexSerializer : JsonSerializer() { + override fun serialize(value: Byte, gen: JsonGenerator, serializers: SerializerProvider?) { + gen.writeString("0x" + hexFormatter.toHexDigits(value)) + } +} + +object UByteToHexSerializer : JsonSerializer() { + override fun serialize(value: UByte, gen: JsonGenerator, serializers: SerializerProvider?) { + gen.writeString("0x" + hexFormatter.toHexDigits(value.toByte())) + } +} + +object ByteArrayToHexDeserializer : JsonDeserializer() { + override fun deserialize(parser: JsonParser, contex: DeserializationContext): ByteArray { + return hexFormatter.parseHex(parser.text.removePrefix("0x")) + } +} + +object ByteToHexDeserializer : JsonDeserializer() { + override fun deserialize(parser: JsonParser, contex: DeserializationContext): Byte { + return hexFormatter.parseHex(parser.text.removePrefix("0x"))[0] + } +} + +object UByteToHexDeserializer : JsonDeserializer() { + override fun deserialize(parser: JsonParser, contex: DeserializationContext): UByte { + return hexFormatter.parseHex(parser.text.removePrefix("0x"))[0].toUByte() + } +} diff --git a/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/InstantISO8601SerDe.kt b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/InstantISO8601SerDe.kt new file mode 100644 index 000000000..3c1d671ec --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/InstantISO8601SerDe.kt @@ -0,0 +1,22 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.core.JsonGenerator +import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.JsonDeserializer +import com.fasterxml.jackson.databind.JsonSerializer +import com.fasterxml.jackson.databind.SerializerProvider +import kotlinx.datetime.Instant + +object InstantISO8601Serializer : JsonSerializer() { + override fun serialize(value: Instant, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toString()) + } +} + +// To uncomment and add the tests when necessary +object InstantISO8601Deserializer : JsonDeserializer() { + override fun deserialize(p: JsonParser, ctxt: DeserializationContext): Instant { + return Instant.parse(p.text) + } +} diff --git a/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/NumbersHexSerDe.kt b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/NumbersHexSerDe.kt new file mode 100644 index 000000000..50496b5cd --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/NumbersHexSerDe.kt @@ -0,0 +1,47 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.core.JsonGenerator +import com.fasterxml.jackson.databind.JsonSerializer +import com.fasterxml.jackson.databind.SerializerProvider +import java.math.BigInteger + +internal fun Number.toHex(): String = "0x" + BigInteger(toString()).toString(16) +internal fun ULong.toHex(): String = "0x" + BigInteger(toString()).toString(16) + +object IntToHexSerializer : JsonSerializer() { + override fun serialize(value: Int, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toHex()) + } +} + +@Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") +object JIntegerToHexSerializer : JsonSerializer() { + override fun serialize(value: Integer, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toHex()) + } +} + +object LongToHexSerializer : JsonSerializer() { + override fun serialize(value: Long, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toHex()) + } +} + +@Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") +object JLongToHexSerializer : JsonSerializer() { + override fun serialize(value: java.lang.Long, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toHex()) + } +} + +object ULongToHexSerializer : JsonSerializer() { + override fun serialize(value: ULong, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toHex()) + } +} + +object BigIntegerToHexSerializer : JsonSerializer() { + override fun serialize(value: BigInteger, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString(value.toHex()) + } +} diff --git a/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/ObjectMappers.kt b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/ObjectMappers.kt new file mode 100644 index 000000000..dcd093ab3 --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/main/kotlin/build/linea/s11n/jackson/ObjectMappers.kt @@ -0,0 +1,36 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.module.SimpleModule +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import kotlinx.datetime.Instant +import java.math.BigInteger + +val ethNumberAsHexSerialisersModule = SimpleModule().apply { + this.addSerializer(Instant::class.java, InstantISO8601Serializer) + this.addDeserializer(Instant::class.java, InstantISO8601Deserializer) + this.addSerializer(Int::class.java, IntToHexSerializer) + this.addSerializer(Integer::class.java, JIntegerToHexSerializer) + this.addSerializer(Long::class.java, LongToHexSerializer) + this.addSerializer(java.lang.Long::class.java, JLongToHexSerializer) + this.addSerializer(ULong::class.java, ULongToHexSerializer) + this.addSerializer(BigInteger::class.java, BigIntegerToHexSerializer) +} + +val ethByteAsHexSerialisersModule = SimpleModule().apply { + this.addSerializer(Byte::class.java, ByteToHexSerializer) + this.addSerializer(UByte::class.java, UByteToHexSerializer) + this.addSerializer(ByteArray::class.java, ByteArrayToHexSerializer) +} + +val ethByteAsHexDeserialisersModule = SimpleModule().apply { + this.addDeserializer(Byte::class.java, ByteToHexDeserializer) + this.addDeserializer(UByte::class.java, UByteToHexDeserializer) + this.addDeserializer(ByteArray::class.java, ByteArrayToHexDeserializer) +} + +val ethApiObjectMapper: ObjectMapper = jacksonObjectMapper() + .registerModules( + ethNumberAsHexSerialisersModule, + ethByteAsHexSerialisersModule + ) diff --git a/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/BytesSerDeTest.kt b/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/BytesSerDeTest.kt new file mode 100644 index 000000000..cbafa2c3b --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/BytesSerDeTest.kt @@ -0,0 +1,160 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue +import net.consensys.decodeHex +import net.consensys.encodeHex +import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +class BytesSerDeTest { + private lateinit var objectMapper: ObjectMapper + + private val jsonObj = """ + { + "nullBytes": null, + "emptyBytes": "0x", + "someBytes": "0x01aaff04", + "listOfByteArray": ["0x01aaff04", "0x01aaff05"], + "nullUByte": null, + "someUByte": "0xaf", + "minUByte": "0x00", + "maxUByte": "0xff", + "nullByte": null, + "someByte": "0xaf", + "minByte": "0x80", + "maxByte": "0x7f" + } + """.trimIndent() + private val objWithBytesFields = SomeObject( + // ByteArray + nullBytes = null, + emptyBytes = byteArrayOf(), + someBytes = "0x01aaff04".decodeHex(), + listOfByteArray = listOf("0x01aaff04", "0x01aaff05").map { it.decodeHex() }, + + // UByte + nullUByte = null, + someUByte = 0xaf.toUByte(), + minUByte = UByte.MIN_VALUE, + maxUByte = UByte.MAX_VALUE, + + // Byte + nullByte = null, + someByte = 0xaf.toByte(), + minByte = Byte.MIN_VALUE, + maxByte = Byte.MAX_VALUE + ) + + @BeforeEach + fun setUp() { + objectMapper = jacksonObjectMapper() + .registerModules(ethByteAsHexSerialisersModule) + .registerModules(ethByteAsHexDeserialisersModule) + } + + @Test + fun bytesSerDe() { + assertThatJson(objectMapper.writeValueAsString(objWithBytesFields)).isEqualTo(jsonObj) + assertThat(objectMapper.readValue(jsonObj)).isEqualTo(objWithBytesFields) + } + + @Test + fun testBytes() { + val list1 = listOf("0x01aaff04", "0x01aaff05").map { it.decodeHex() } + val list2 = listOf("0x01aaff04", "0x01aaff05", "0x01aaff06").map { it.decodeHex() } + list1.zip(list2).also { println(it) } + println(list1.zip(list2).all { (arr1, arr2) -> arr1.contentEquals(arr2) }) + + println(list1 == list2) + println(list1 != list2) + } + + private data class SomeObject( + // ByteArray + val nullBytes: ByteArray?, + val emptyBytes: ByteArray, + val someBytes: ByteArray, + val listOfByteArray: List, + + // UByte + val nullUByte: UByte?, + val someUByte: UByte, + val minUByte: UByte, + val maxUByte: UByte, + // Byte + val nullByte: Byte?, + val someByte: Byte, + val minByte: Byte, + val maxByte: Byte + ) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + + other as SomeObject + + if (nullBytes != null) { + if (other.nullBytes == null) return false + if (!nullBytes.contentEquals(other.nullBytes)) return false + } else if (other.nullBytes != null) return false + if (!emptyBytes.contentEquals(other.emptyBytes)) return false + if (!someBytes.contentEquals(other.someBytes)) return false + if (!contentEquals(listOfByteArray, other.listOfByteArray)) return false + if (nullUByte != other.nullUByte) return false + if (someUByte != other.someUByte) return false + if (minUByte != other.minUByte) return false + if (maxUByte != other.maxUByte) return false + if (nullByte != other.nullByte) return false + if (someByte != other.someByte) return false + if (minByte != other.minByte) return false + if (maxByte != other.maxByte) return false + + return true + } + + override fun hashCode(): Int { + var result = nullBytes?.contentHashCode() ?: 0 + result = 31 * result + emptyBytes.contentHashCode() + result = 31 * result + someBytes.contentHashCode() + result = 31 * result + listOfByteArray.hashCode() + result = 31 * result + (nullUByte?.hashCode() ?: 0) + result = 31 * result + someUByte.hashCode() + result = 31 * result + minUByte.hashCode() + result = 31 * result + maxUByte.hashCode() + result = 31 * result + (nullByte ?: 0) + result = 31 * result + someByte + result = 31 * result + minByte + result = 31 * result + maxByte + return result + } + + override fun toString(): String { + return "SomeObject(" + + "nullBytes=${nullBytes?.contentToString()}, " + + "emptyBytes=${emptyBytes.contentToString()}, " + + "someByte=${someBytes.contentToString()}, " + + "listOfByteArray=${listOfByteArray.joinToString(",", "[", "]") { it.encodeHex() }}, " + + "nullUByte=$nullUByte, " + + "someUByte=$someUByte, " + + "minUByte=$minUByte, " + + "maxUByte=$maxUByte, " + + "nullByte=$nullByte, " + + "someByte=$someByte, " + + "minByte=$minByte, " + + "maxByte=$maxByte" + + ")" + } + } + + companion object { + fun contentEquals(list1: List, list2: List): Boolean { + if (list1.size != list2.size) return false + + return list1.zip(list2).all { (arr1, arr2) -> arr1.contentEquals(arr2) } + } + } +} diff --git a/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/InstantISO8601SerDeTest.kt b/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/InstantISO8601SerDeTest.kt new file mode 100644 index 000000000..21e5d732a --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/InstantISO8601SerDeTest.kt @@ -0,0 +1,65 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.databind.ObjectMapper +import kotlinx.datetime.Instant +import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +class InstantISO8601SerDeTest { + private lateinit var objectMapper: ObjectMapper + + @BeforeEach + fun setUp() { + objectMapper = ethApiObjectMapper + } + + @Test + fun instantSerialization() { + data class SomeObject( + // Int + val instantNull: Instant? = null, + val instantUTC: Instant = Instant.parse("2021-01-02T09:00:45Z"), + val instantUTCPlus: Instant = Instant.parse("2021-01-02T09:00:45+01:30"), + val instantUTCMinus: Instant = Instant.parse("2021-01-02T09:00:45-01:30") + ) + + val json = objectMapper.writeValueAsString(SomeObject()) + assertThatJson(json).isEqualTo( + """ + { + "instantNull": null, + "instantUTC": "2021-01-02T09:00:45Z", + "instantUTCPlus": "2021-01-02T07:30:45Z", + "instantUTCMinus": "2021-01-02T10:30:45Z" + } + """.trimIndent() + ) + } + + @Test + fun instantSerDeDeSerialization() { + data class SomeObject( + // Int + val instantNull: Instant? = null, + val instantUTC: Instant = Instant.parse("2021-01-02T09:00:45Z"), + val instantUTCPlus: Instant = Instant.parse("2021-01-02T09:00:45+01:30"), + val instantUTCMinus: Instant = Instant.parse("2021-01-02T09:00:45-01:30") + ) + + val expectedJson = """ + { + "instantNull": null, + "instantUTC": "2021-01-02T09:00:45Z", + "instantUTCPlus": "2021-01-02T07:30:45Z", + "instantUTCMinus": "2021-01-02T10:30:45Z" + } + """.trimIndent() + + // assert serialization + assertThatJson(objectMapper.writeValueAsString(SomeObject())).isEqualTo(expectedJson) + + // assert deserialization + assertThatJson(objectMapper.readValue(expectedJson, SomeObject::class.java)).isEqualTo(SomeObject()) + } +} diff --git a/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/NumbersSerDeTest.kt b/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/NumbersSerDeTest.kt new file mode 100644 index 000000000..bba6e82dd --- /dev/null +++ b/jvm-libs/generic/serialization/jackson/src/test/kotlin/build/linea/s11n/jackson/NumbersSerDeTest.kt @@ -0,0 +1,80 @@ +package build.linea.s11n.jackson + +import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.ObjectMapper +import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import java.math.BigInteger + +class NumbersSerDeTest { + private lateinit var objectMapper: ObjectMapper + + @BeforeEach + fun setUp() { + objectMapper = ethApiObjectMapper + } + + @Test + fun numbersSerialization() { + data class SomeObject( + // Int + val intNull: Int? = null, + val intZero: Int = 0, + val intMaxValue: Int = Int.MAX_VALUE, + val intSomeValue: Int = 0xff00aa, + + // Long + val longNull: Long? = null, + val longZero: Long = 0, + val longMaxValue: Long = Long.MAX_VALUE, + val longSomeValue: Long = 0xff00aa, + + // ULong + // jackson has a bug and serializes any cAmelCase as camelCase, need to set it with @JsonProperty + // it's only on 1st character, so we can't use uLongNull, uLongZero, without annotation etc. + @get:JsonProperty("uLongNull") val uLongNull: ULong? = null, + @get:JsonProperty("uLongZero") val uLongZero: ULong = 0uL, + @get:JsonProperty("uLongMaxValue") val uLongMaxValue: ULong = ULong.MAX_VALUE, + @get:JsonProperty("uLongSomeValue") val uLongSomeValue: ULong = 0xff00aa.toULong(), + + // BigInteger + val bigIntegerNull: BigInteger? = null, + val bigIntegerZero: BigInteger = BigInteger.ZERO, + val bigIntegerSomeValue: BigInteger = BigInteger.valueOf(0xff00aaL), + + // nested Structures + val listOfInts: List = listOf(1, 10), + val listOfLongs: List = listOf(1, 10), + val listOfULongs: List = listOf(1UL, 10UL), + val listOfBigIntegers: List = listOf(1L, 10L).map(BigInteger::valueOf) + ) + + val json = objectMapper.writeValueAsString(SomeObject()) + assertThatJson(json).isEqualTo( + """ + { + "intNull": null, + "intZero": "0x0", + "intMaxValue": "0x7fffffff", + "intSomeValue": "0xff00aa", + "longNull": null, + "longZero": "0x0", + "longMaxValue": "0x7fffffffffffffff", + "longSomeValue": "0xff00aa", + "uLongNull": null, + "uLongZero": "0x0", + "uLongMaxValue": "0xffffffffffffffff", + "uLongSomeValue": "0xff00aa", + "bigIntegerNull": null, + "bigIntegerZero": "0x0", + "bigIntegerSomeValue": "0xff00aa", + "listOfInts": ["0x1", "0xa"], + "listOfLongs": ["0x1", "0xa"], + "listOfULongs": ["0x1", "0xa"], + "listOfBigIntegers": ["0x1", "0xa"] + } + """.trimIndent() + ) + } +} diff --git a/settings.gradle b/settings.gradle index 674fb5b9b..73082e51f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -21,6 +21,7 @@ include 'jvm-libs:metrics:micrometer' include 'jvm-libs:teku-execution-client' include 'jvm-libs:testing:l1-blob-and-proof-submission' include 'jvm-libs:testing:file-system' +include 'jvm-libs:generic:serialization:jackson' include 'coordinator:app' include 'coordinator:core' include 'coordinator:utilities' From 9fc7fa6da9aa16fc9d5583ffa05caed2106667a3 Mon Sep 17 00:00:00 2001 From: Pedro Novais <1478752+jpnovais@users.noreply.github.com> Date: Fri, 18 Oct 2024 16:33:23 +0100 Subject: [PATCH 3/6] JSON-RPC client improvements (#195) * adds json-rpc client V2 with simpler interface Signed-off-by: Pedro Novais <1478752+jpnovais@users.noreply.github.com> Co-authored-by: jonesho <81145364+jonesho@users.noreply.github.com> Co-authored-by: Roman Vaseev <4833306+Filter94@users.noreply.github.com> --- jvm-libs/json-rpc/build.gradle | 3 + .../linea/jsonrpc/JsonRpcMessageProcessor.kt | 5 + .../consensys/linea/jsonrpc/JsonRpcRequest.kt | 24 +- .../linea/jsonrpc/JsonRpcResponse.kt | 22 +- .../linea/jsonrpc/client/JsonRpcClient.kt | 37 +- .../jsonrpc/client/JsonRpcRequestFanOut.kt | 2 +- .../jsonrpc/client/JsonRpcRequestRetryer.kt | 2 +- .../jsonrpc/client/JsonRpcRequestRetryerV2.kt | 109 +++ .../linea/jsonrpc/client/JsonRpcV2Client.kt | 34 + .../jsonrpc/client/JsonRpcV2ClientImpl.kt | 28 + .../linea/jsonrpc/client/ObjectMappers.kt | 26 + .../jsonrpc/client/SequentialIdSupplier.kt | 19 + .../jsonrpc/client/VertxHttpJsonRpcClient.kt | 79 +- .../client/VertxHttpJsonRpcClientFactory.kt | 81 ++- .../jsonrpc/client/JsonRpcV2ClientImplTest.kt | 672 ++++++++++++++++++ .../client/VertxHttpJsonRpcClientTest.kt | 99 ++- .../linea/traces/RawJsonTracesConflator.kt | 38 +- 17 files changed, 1173 insertions(+), 107 deletions(-) create mode 100644 jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryerV2.kt create mode 100644 jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2Client.kt create mode 100644 jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImpl.kt create mode 100644 jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/ObjectMappers.kt create mode 100644 jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/SequentialIdSupplier.kt create mode 100644 jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImplTest.kt diff --git a/jvm-libs/json-rpc/build.gradle b/jvm-libs/json-rpc/build.gradle index 16d2ef7dd..9a06d380d 100644 --- a/jvm-libs/json-rpc/build.gradle +++ b/jvm-libs/json-rpc/build.gradle @@ -6,6 +6,8 @@ plugins { dependencies { implementation project(":jvm-libs:metrics:micrometer") implementation project(":jvm-libs:future-extensions") + implementation project(":jvm-libs:kotlin-extensions") + implementation project(":jvm-libs:generic:serialization:jackson") implementation "com.fasterxml.jackson.core:jackson-annotations:${libs.versions.jackson.get()}" api "com.fasterxml.jackson.core:jackson-databind:${libs.versions.jackson.get()}" implementation "com.fasterxml.jackson.module:jackson-module-kotlin:${libs.versions.jackson.get()}" @@ -22,6 +24,7 @@ dependencies { testImplementation "io.rest-assured:rest-assured:${libs.versions.restassured.get()}" testImplementation "io.rest-assured:json-schema-validator:${libs.versions.restassured.get()}" testImplementation "com.github.tomakehurst:wiremock-jre8:${libs.versions.wiremock.get()}" + testImplementation "net.javacrumbs.json-unit:json-unit-assertj:${libs.versions.jsonUnit.get()}" } jar { diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcMessageProcessor.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcMessageProcessor.kt index 0b708a5c0..ebe6c8109 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcMessageProcessor.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcMessageProcessor.kt @@ -1,5 +1,6 @@ package net.consensys.linea.jsonrpc +import com.fasterxml.jackson.module.kotlin.registerKotlinModule import com.github.michaelbull.result.Err import com.github.michaelbull.result.Ok import com.github.michaelbull.result.Result @@ -19,6 +20,7 @@ import io.vertx.core.json.DecodeException import io.vertx.core.json.Json import io.vertx.core.json.JsonArray import io.vertx.core.json.JsonObject +import io.vertx.core.json.jackson.DatabindCodec import io.vertx.ext.auth.User import net.consensys.linea.metrics.micrometer.DynamicTagTimerCapture import net.consensys.linea.metrics.micrometer.SimpleTimerCapture @@ -51,6 +53,9 @@ class JsonRpcMessageProcessor( private val meterRegistry: MeterRegistry, private val requestParser: JsonRpcRequestParser = Companion::parseRequest ) : JsonRpcMessageHandler { + init { + DatabindCodec.mapper().registerKotlinModule() + } private val log: Logger = LogManager.getLogger(this.javaClass) private val counterBuilder = Counter.builder("jsonrpc.counter") override fun invoke(user: User?, messageJsonStr: String): Future = diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcRequest.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcRequest.kt index ba68dfebc..1ac494b21 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcRequest.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcRequest.kt @@ -1,7 +1,6 @@ package net.consensys.linea.jsonrpc import com.fasterxml.jackson.annotation.JsonIgnore -import com.fasterxml.jackson.annotation.JsonProperty import java.util.StringJoiner interface JsonRpcRequest { @@ -20,11 +19,18 @@ interface JsonRpcRequest { } } +internal data class JsonRpcRequestData( + override val jsonrpc: String, + override val id: Any, + override val method: String, + override val params: Any +) : JsonRpcRequest + data class JsonRpcRequestListParams( - @JsonProperty("jsonrpc") override val jsonrpc: String, - @JsonProperty("id") override val id: Any, - @JsonProperty("method") override val method: String, - @JsonProperty("params") override val params: List + override val jsonrpc: String, + override val id: Any, + override val method: String, + override val params: List ) : JsonRpcRequest { override fun toString(): String { return StringJoiner(", ", JsonRpcRequestListParams::class.java.simpleName + "[", "]") @@ -37,10 +43,10 @@ data class JsonRpcRequestListParams( } data class JsonRpcRequestMapParams( - @JsonProperty("jsonrpc") override val jsonrpc: String, - @JsonProperty("id") override val id: Any, - @JsonProperty("method") override val method: String, - @JsonProperty("params") override val params: Map + override val jsonrpc: String, + override val id: Any, + override val method: String, + override val params: Map ) : JsonRpcRequest { override fun toString(): String { return StringJoiner(", ", JsonRpcRequestMapParams::class.java.simpleName + "[", "]") diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcResponse.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcResponse.kt index c5ec83f52..6fbebe302 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcResponse.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/JsonRpcResponse.kt @@ -61,6 +61,7 @@ data class JsonRpcErrorResponse( fun internalError(id: Any, data: Any?): JsonRpcErrorResponse { return JsonRpcErrorResponse(id, JsonRpcError.internalError(data)) } + fun invalidParams(id: Any, message: String?): JsonRpcErrorResponse { return JsonRpcErrorResponse(id, JsonRpcError.invalidMethodParameter(message)) } @@ -74,8 +75,10 @@ data class JsonRpcError( @JsonProperty("message") val message: String, @JsonProperty("data") val data: Any? = null ) { + // inlining for better stacktrace + @Suppress("NOTHING_TO_INLINE") + inline fun asException() = JsonRpcErrorResponseException(code, message, data) - fun asException() = JsonRpcErrorException("Code: $code, message: '$message'") companion object { @JvmStatic fun invalidMethodParameter(message: String?): JsonRpcError = @@ -88,14 +91,25 @@ data class JsonRpcError( fun invalidMethodParameter(message: String, data: Any): JsonRpcError = JsonRpcError(JsonRpcErrorCode.INVALID_PARAMS.code, message, data) - @JvmStatic fun internalError(): JsonRpcError = JsonRpcErrorCode.INTERNAL_ERROR.toErrorObject() + @JvmStatic + fun internalError(): JsonRpcError = JsonRpcErrorCode.INTERNAL_ERROR.toErrorObject() @JvmStatic fun internalError(data: Any?): JsonRpcError = JsonRpcErrorCode.INTERNAL_ERROR.toErrorObject(data) - @JvmStatic fun unauthorized(): JsonRpcError = JsonRpcErrorCode.UNAUTHORIZED.toErrorObject() + @JvmStatic + fun unauthorized(): JsonRpcError = JsonRpcErrorCode.UNAUTHORIZED.toErrorObject() } } -class JsonRpcErrorException(override val message: String) : Exception(message) +class JsonRpcErrorException( + override val message: String?, + val httpStatusCode: Int? = null +) : RuntimeException(message) + +class JsonRpcErrorResponseException( + val rpcErrorCode: Int, + val rpcErrorMessage: String, + val rpcErrorData: Any? = null +) : RuntimeException("code=$rpcErrorCode message=$rpcErrorMessage errorData=$rpcErrorData") diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcClient.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcClient.kt index 0edb93bc8..3cfcddaf9 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcClient.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcClient.kt @@ -1,27 +1,56 @@ package net.consensys.linea.jsonrpc.client +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.JsonNodeType import com.github.michaelbull.result.Ok import com.github.michaelbull.result.Result import io.vertx.core.Future +import io.vertx.core.json.JsonArray +import io.vertx.core.json.JsonObject import net.consensys.linea.jsonrpc.JsonRpcErrorResponse import net.consensys.linea.jsonrpc.JsonRpcRequest import net.consensys.linea.jsonrpc.JsonRpcSuccessResponse -fun identityMapper(value: Any?): Any? = value +fun toPrimitiveOrJacksonJsonNode(value: Any?): Any? = value + +@Suppress("UNCHECKED_CAST") +fun toPrimitiveOrVertxJson(value: Any?): Any? { + if (value == null) { + return null + } + return when (value) { + is String -> value + is Number -> value + is Boolean -> value + is JsonNode -> { + when (value.nodeType) { + JsonNodeType.STRING, JsonNodeType.NUMBER, JsonNodeType.BOOLEAN, JsonNodeType.NULL -> + value + .toPrimitiveOrJsonNode() + + JsonNodeType.OBJECT -> JsonObject(objectMapper.convertValue(value, Map::class.java) as Map) + JsonNodeType.ARRAY -> JsonArray(objectMapper.convertValue(value, List::class.java) as List) + else -> throw IllegalArgumentException("Unsupported JsonNodeType: ${value.nodeType}") + } + } + + else -> throw IllegalArgumentException("Unsupported type: ${value::class.java}") + } +} interface JsonRpcClient { fun makeRequest( request: JsonRpcRequest, - resultMapper: (Any?) -> Any? = ::identityMapper + resultMapper: (Any?) -> Any? = ::toPrimitiveOrVertxJson // to keep backward compatibility ): Future> } -fun isResultOk(result: Result): Boolean = result is Ok +fun isResultOk(result: Result): Boolean = result is Ok interface JsonRpcClientWithRetries : JsonRpcClient { fun makeRequest( request: JsonRpcRequest, - resultMapper: (Any?) -> Any? = ::identityMapper, + resultMapper: (Any?) -> Any? = ::toPrimitiveOrVertxJson, // to keep backward compatibility stopRetriesPredicate: (result: Result) -> Boolean = ::isResultOk ): Future> } diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestFanOut.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestFanOut.kt index 15bbca706..589205ad1 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestFanOut.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestFanOut.kt @@ -39,7 +39,7 @@ class JsonRpcRequestFanOut( fun fanoutRequest( request: JsonRpcRequest, - resultMapper: (Any?) -> Any? = ::identityMapper + resultMapper: (Any?) -> Any? = ::toPrimitiveOrVertxJson ): Future>> { return Future .all(targets.map { it.makeRequest(request, resultMapper) }) diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryer.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryer.kt index 12b15142b..0bb637823 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryer.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryer.kt @@ -45,7 +45,7 @@ class JsonRpcRequestRetryer( private val vertx: Vertx, private val delegate: JsonRpcClient, private val config: Config, - private val requestObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, + private val requestObjectMapper: ObjectMapper = objectMapper, private val log: Logger = LogManager.getLogger(JsonRpcRequestRetryer::class.java), private val failuresLogLevel: Level = Level.WARN ) : JsonRpcClientWithRetries { diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryerV2.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryerV2.kt new file mode 100644 index 000000000..0182324b0 --- /dev/null +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcRequestRetryerV2.kt @@ -0,0 +1,109 @@ +package net.consensys.linea.jsonrpc.client + +import com.fasterxml.jackson.databind.ObjectMapper +import com.github.michaelbull.result.Err +import com.github.michaelbull.result.Ok +import com.github.michaelbull.result.Result +import com.github.michaelbull.result.map +import com.github.michaelbull.result.mapError +import com.github.michaelbull.result.onFailure +import io.vertx.core.Vertx +import net.consensys.linea.async.AsyncRetryer +import net.consensys.linea.async.RetriedExecutionException +import net.consensys.linea.async.toSafeFuture +import net.consensys.linea.jsonrpc.JsonRpcErrorResponse +import net.consensys.linea.jsonrpc.JsonRpcRequest +import net.consensys.linea.jsonrpc.JsonRpcSuccessResponse +import org.apache.logging.log4j.Level +import org.apache.logging.log4j.LogManager +import org.apache.logging.log4j.Logger +import tech.pegasys.teku.infrastructure.async.SafeFuture +import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference +import java.util.function.Predicate + +class JsonRpcRequestRetryerV2( + private val vertx: Vertx, + private val delegate: JsonRpcClient, + private val requestRetry: RequestRetryConfig, + private val requestObjectMapper: ObjectMapper = objectMapper, + private val shallRetryRequestsClientBasePredicate: Predicate>, + private val log: Logger = LogManager.getLogger(JsonRpcRequestRetryer::class.java), + private val failuresLogLevel: Level = Level.WARN +) { + fun makeRequest( + request: JsonRpcRequest, + shallRetryRequestPredicate: Predicate>, + resultMapper: (Any?) -> T + ): SafeFuture { + return makeRequestWithRetryer(request, resultMapper, shallRetryRequestPredicate) + } + + private fun shallWarnFailureRetries(retries: Int): Boolean { + return requestRetry.failuresWarningThreshold > 0u && + retries > 0 && + (retries % requestRetry.failuresWarningThreshold.toInt()) == 0 + } + + private fun makeRequestWithRetryer( + request: JsonRpcRequest, + resultMapper: (Any?) -> T, + shallRetryRequestPredicate: Predicate> + ): SafeFuture { + val lastException = AtomicReference() + val retriesCount = AtomicInteger(0) + val requestPredicate = Predicate> { result -> + log.info("result: {}", result) + shallRetryRequestsClientBasePredicate.test(result) || shallRetryRequestPredicate.test(result) + } + + return AsyncRetryer.retry( + vertx = vertx, + backoffDelay = requestRetry.backoffDelay, + maxRetries = requestRetry.maxRetries?.toInt(), + timeout = requestRetry.timeout, + stopRetriesPredicate = { result: Result -> + result.onFailure(lastException::set) + !requestPredicate.test(result) + } + ) { + if (shallWarnFailureRetries(retriesCount.get())) { + log.log( + failuresLogLevel, + "Request '{}' already retried {} times. lastError={}", + requestObjectMapper.writeValueAsString(request), + retriesCount.get(), + lastException.get() + ) + } + retriesCount.incrementAndGet() + delegate.makeRequest(request, resultMapper).toSafeFuture().thenApply { unfoldResultValueOrException(it) } + .exceptionally { th -> + if (th is Error || th.cause is Error) { + // Very serious JVM error, we should stop retrying anyway + throw th + } else { + Err(th.cause ?: th) + } + } + }.handleComposed { result, throwable -> + when { + result is Ok -> SafeFuture.completedFuture(result.value) + result is Err -> SafeFuture.failedFuture(result.error) + throwable != null && throwable is RetriedExecutionException -> SafeFuture.failedFuture(lastException.get()) + else -> SafeFuture.failedFuture(throwable) + } + } + } + + companion object { + fun unfoldResultValueOrException( + response: Result + ): Result { + @Suppress("UNCHECKED_CAST") + return response + .map { it.result as T } + .mapError { it.error.asException() } + } + } +} diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2Client.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2Client.kt new file mode 100644 index 000000000..20fef8440 --- /dev/null +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2Client.kt @@ -0,0 +1,34 @@ +package net.consensys.linea.jsonrpc.client + +import com.github.michaelbull.result.Result +import tech.pegasys.teku.infrastructure.async.SafeFuture +import java.util.function.Predicate + +/** + * JSON-RPC client that supports JSON-RPC v2.0. + * It will automatically generate the request id and retry requests when JSON-RPC errors are received. + * Please override default stopRetriesPredicate to customize the retry logic. + * + * JSON-RPC result/error.data serialization is done automatically to Jackson JsonNode or primitive types. + */ +interface JsonRpcV2Client { + /** + * Makes a JSON-RPC request. + * @param method The method to call. + * @param params The parameters to pass to the method. It can be a List, a Map or a Pojo. + * @param shallRetryRequestPredicate predicate to evaluate request retrying. It defaults to never retrying. + * @param resultMapper Mapper to apply to successful JSON-RPC result. + * the result is primary type (String, Number, Boolean, null) or (jackson's JsonNode or vertx JsonObject/JsonArray) + * The underlying type will depend on the serialization configured on the concrete implementation. + * @return A future that + * - when success - resolves with mapped result + * - when JSON-RPC error - rejects with JsonRpcErrorException with corresponding error code, message and data + * - when other error - rejects with underlying exception + */ + fun makeRequest( + method: String, + params: Any, // List, Map, Pojo + shallRetryRequestPredicate: Predicate> = Predicate { false }, + resultMapper: (Any?) -> T + ): SafeFuture +} diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImpl.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImpl.kt new file mode 100644 index 000000000..ac73bc1c2 --- /dev/null +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImpl.kt @@ -0,0 +1,28 @@ +package net.consensys.linea.jsonrpc.client + +import com.github.michaelbull.result.Result +import net.consensys.linea.jsonrpc.JsonRpcRequestData +import tech.pegasys.teku.infrastructure.async.SafeFuture +import java.util.function.Predicate +import java.util.function.Supplier + +internal class JsonRpcV2ClientImpl( + private val delegate: JsonRpcRequestRetryerV2, + private val idSupplier: Supplier +) : JsonRpcV2Client { + + override fun makeRequest( + method: String, + params: Any, + shallRetryRequestPredicate: Predicate>, + resultMapper: (Any?) -> T + ): SafeFuture { + val request = JsonRpcRequestData(jsonrpc = "2.0", id = idSupplier.get(), method, params) + + return delegate.makeRequest( + request = request, + shallRetryRequestPredicate = shallRetryRequestPredicate, + resultMapper = resultMapper + ) + } +} diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/ObjectMappers.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/ObjectMappers.kt new file mode 100644 index 000000000..f718cf0f0 --- /dev/null +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/ObjectMappers.kt @@ -0,0 +1,26 @@ +package net.consensys.linea.jsonrpc.client + +import build.linea.s11n.jackson.ethByteAsHexSerialisersModule +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.JsonNodeType +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import io.vertx.core.json.jackson.VertxModule + +internal val objectMapper = jacksonObjectMapper() + .registerModules(VertxModule()) + .registerModules(ethByteAsHexSerialisersModule) + +fun JsonNodeType.isPrimitive(): Boolean { + return when (this) { + JsonNodeType.STRING, JsonNodeType.NUMBER, JsonNodeType.BOOLEAN, JsonNodeType.NULL -> true + else -> false + } +} + +fun JsonNode.toPrimitiveOrJsonNode(): Any? { + return if (this.nodeType.isPrimitive()) { + objectMapper.convertValue(this, Any::class.java) + } else { + this + } +} diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/SequentialIdSupplier.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/SequentialIdSupplier.kt new file mode 100644 index 000000000..338d0b302 --- /dev/null +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/SequentialIdSupplier.kt @@ -0,0 +1,19 @@ +package net.consensys.linea.jsonrpc.client + +import java.util.concurrent.atomic.AtomicLong +import java.util.function.Supplier +import javax.annotation.concurrent.ThreadSafe + +@ThreadSafe +class SequentialIdSupplier : Supplier { + private var id = AtomicLong(0) + + // if application makes 1_000 requests per second, it will take 292,277,026,596 years of uptime to overflow + override fun get(): Any { + return id.incrementAndGet() + } + + companion object { + val singleton = SequentialIdSupplier() + } +} diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClient.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClient.kt index 7bd5cf0b2..42eb78843 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClient.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClient.kt @@ -1,10 +1,7 @@ package net.consensys.linea.jsonrpc.client -import com.fasterxml.jackson.core.JsonGenerator -import com.fasterxml.jackson.databind.JsonSerializer import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.databind.SerializerProvider -import com.fasterxml.jackson.databind.module.SimpleModule +import com.fasterxml.jackson.module.kotlin.contains import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.github.michaelbull.result.Err import com.github.michaelbull.result.Ok @@ -16,24 +13,24 @@ import io.vertx.core.http.HttpClient import io.vertx.core.http.HttpClientResponse import io.vertx.core.http.HttpMethod import io.vertx.core.http.RequestOptions -import io.vertx.core.json.JsonObject -import io.vertx.core.json.jackson.VertxModule import net.consensys.linea.jsonrpc.JsonRpcError +import net.consensys.linea.jsonrpc.JsonRpcErrorException import net.consensys.linea.jsonrpc.JsonRpcErrorResponse import net.consensys.linea.jsonrpc.JsonRpcRequest +import net.consensys.linea.jsonrpc.JsonRpcRequestData import net.consensys.linea.jsonrpc.JsonRpcSuccessResponse import net.consensys.linea.metrics.micrometer.SimpleTimerCapture import org.apache.logging.log4j.Level import org.apache.logging.log4j.LogManager import org.apache.logging.log4j.Logger import java.net.URL -import java.util.HexFormat +@Suppress("UNCHECKED_CAST") class VertxHttpJsonRpcClient( private val httpClient: HttpClient, private val endpoint: URL, private val meterRegistry: MeterRegistry, - private val requestObjectMapper: ObjectMapper = objectMapper, + private val requestParamsObjectMapper: ObjectMapper = objectMapper, private val responseObjectMapper: ObjectMapper = objectMapper, private val log: Logger = LogManager.getLogger(VertxHttpJsonRpcClient::class.java), private val requestResponseLogLevel: Level = Level.TRACE, @@ -44,11 +41,22 @@ class VertxHttpJsonRpcClient( setAbsoluteURI(endpoint) } + private fun serializeRequest(request: JsonRpcRequest): String { + return requestEnvelopeObjectMapper.writeValueAsString( + JsonRpcRequestData( + jsonrpc = request.jsonrpc, + id = request.id, + method = request.method, + params = requestParamsObjectMapper.valueToTree(request.params) + ) + ) + } + override fun makeRequest( request: JsonRpcRequest, resultMapper: (Any?) -> Any? ): Future> { - val json = requestObjectMapper.writeValueAsString(request) + val json = serializeRequest(request) return httpClient.request(requestOptions).flatMap { httpClientRequest -> httpClientRequest.putHeader("Content-Type", "application/json") @@ -67,8 +75,10 @@ class VertxHttpJsonRpcClient( responseBody = bodyBuffer.toString().lines().firstOrNull() ?: "" ) Future.failedFuture( - Exception( - "HTTP errorCode=${response.statusCode()}, message=${response.statusMessage()}" + JsonRpcErrorException( + message = + "HTTP errorCode=${response.statusCode()}, message=${response.statusMessage()}", + httpStatusCode = response.statusCode() ) ) } @@ -99,37 +109,41 @@ class VertxHttpJsonRpcClient( .flatMap { bodyBuffer: Buffer -> responseBody = bodyBuffer.toString() try { - @Suppress("UNCHECKED_CAST") - val jsonResponse = (responseObjectMapper.readValue(responseBody, Map::class.java) as Map) - .let(::JsonObject) + val jsonResponse = responseObjectMapper.readTree(responseBody) + val responseId = responseObjectMapper.convertValue(jsonResponse.get("id"), Any::class.java) val response = when { - jsonResponse.containsKey("result") -> + jsonResponse.contains("result") -> { Ok( JsonRpcSuccessResponse( - jsonResponse.getValue("id"), - resultMapper(jsonResponse.getValue("result")) + responseId, + resultMapper(jsonResponse.get("result").toPrimitiveOrJsonNode()) ) ) + } - jsonResponse.containsKey("error") -> { + jsonResponse.contains("error") -> { isError = true - Err( - JsonRpcErrorResponse( - jsonResponse.getValue("id"), - jsonResponse.getJsonObject("error").mapTo(JsonRpcError::class.java) - ) + val errorResponse = JsonRpcErrorResponse( + responseId, + responseObjectMapper.treeToValue(jsonResponse["error"], JsonRpcError::class.java) ) + Err(errorResponse) } else -> throw IllegalArgumentException("Invalid JSON-RPC response without result or error") } - Future.succeededFuture(response) + Future.succeededFuture>(response) } catch (e: Throwable) { isError = true when (e) { is IllegalArgumentException -> Future.failedFuture(e) - else -> Future.failedFuture(IllegalArgumentException("Invalid JSON-RPC response.", e)) + else -> Future.failedFuture( + IllegalArgumentException( + "Error parsing JSON-RPC response: message=${e.message}", + e + ) + ) } } } @@ -185,19 +199,6 @@ class VertxHttpJsonRpcClient( } companion object { - val objectMapper = jacksonObjectMapper() - .registerModules(VertxModule()) - .registerModules( - SimpleModule().apply { - this.addSerializer(ByteArray::class.java, ByteArrayToHexStringSerializer()) - } - ) - } -} - -class ByteArrayToHexStringSerializer : JsonSerializer() { - private val hexFormatter = HexFormat.of() - override fun serialize(value: ByteArray?, gen: JsonGenerator?, serializers: SerializerProvider?) { - gen?.writeString(value?.let { "0x" + hexFormatter.formatHex(it) }) + private val requestEnvelopeObjectMapper: ObjectMapper = jacksonObjectMapper() } } diff --git a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientFactory.kt b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientFactory.kt index ccdaa9873..21687a62d 100644 --- a/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientFactory.kt +++ b/jvm-libs/json-rpc/src/main/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientFactory.kt @@ -1,6 +1,8 @@ package net.consensys.linea.jsonrpc.client import com.fasterxml.jackson.databind.ObjectMapper +import com.github.michaelbull.result.Err +import com.github.michaelbull.result.Result import io.micrometer.core.instrument.MeterRegistry import io.vertx.core.Vertx import io.vertx.core.http.HttpClientOptions @@ -9,19 +11,22 @@ import org.apache.logging.log4j.Level import org.apache.logging.log4j.LogManager import org.apache.logging.log4j.Logger import java.net.URL +import java.util.function.Predicate +import java.util.function.Supplier class VertxHttpJsonRpcClientFactory( private val vertx: Vertx, private val meterRegistry: MeterRegistry, private val requestResponseLogLevel: Level = Level.TRACE, - private val failuresLogLevel: Level = Level.DEBUG + private val failuresLogLevel: Level = Level.DEBUG, + private val requestIdSupplier: Supplier = SequentialIdSupplier.singleton ) { fun create( endpoint: URL, maxPoolSize: Int? = null, httpVersion: HttpVersion? = null, - requestObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, - responseObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, + requestObjectMapper: ObjectMapper = objectMapper, + responseObjectMapper: ObjectMapper = objectMapper, log: Logger = LogManager.getLogger(VertxHttpJsonRpcClient::class.java), requestResponseLogLevel: Level = this.requestResponseLogLevel, failuresLogLevel: Level = this.failuresLogLevel @@ -39,7 +44,7 @@ class VertxHttpJsonRpcClientFactory( endpoint, meterRegistry, log = log, - requestObjectMapper = requestObjectMapper, + requestParamsObjectMapper = requestObjectMapper, responseObjectMapper = responseObjectMapper, requestResponseLogLevel = requestResponseLogLevel, failuresLogLevel = failuresLogLevel @@ -50,8 +55,8 @@ class VertxHttpJsonRpcClientFactory( endpoints: Set, maxInflightRequestsPerClient: UInt, httpVersion: HttpVersion? = null, - requestObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, - responseObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, + requestObjectMapper: ObjectMapper = objectMapper, + responseObjectMapper: ObjectMapper = objectMapper, log: Logger = LogManager.getLogger(VertxHttpJsonRpcClient::class.java), requestResponseLogLevel: Level = this.requestResponseLogLevel, failuresLogLevel: Level = this.failuresLogLevel @@ -79,8 +84,8 @@ class VertxHttpJsonRpcClientFactory( retryConfig: RequestRetryConfig, methodsToRetry: Set, httpVersion: HttpVersion? = null, - requestObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, - responseObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, + requestObjectMapper: ObjectMapper = objectMapper, + responseObjectMapper: ObjectMapper = objectMapper, log: Logger = LogManager.getLogger(VertxHttpJsonRpcClient::class.java), requestResponseLogLevel: Level = this.requestResponseLogLevel, failuresLogLevel: Level = this.failuresLogLevel @@ -114,8 +119,8 @@ class VertxHttpJsonRpcClientFactory( retryConfig: RequestRetryConfig, methodsToRetry: Set, httpVersion: HttpVersion? = null, - requestObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, - responseObjectMapper: ObjectMapper = VertxHttpJsonRpcClient.objectMapper, + requestObjectMapper: ObjectMapper = objectMapper, + responseObjectMapper: ObjectMapper = objectMapper, log: Logger = LogManager.getLogger(VertxHttpJsonRpcClient::class.java), requestResponseLogLevel: Level = this.requestResponseLogLevel, failuresLogLevel: Level = this.failuresLogLevel @@ -143,4 +148,60 @@ class VertxHttpJsonRpcClientFactory( log = log ) } + + fun createV2( + vertx: Vertx, + endpoints: Set, + maxInflightRequestsPerClient: UInt? = null, + retryConfig: RequestRetryConfig, + httpVersion: HttpVersion? = null, + requestObjectMapper: ObjectMapper = objectMapper, + responseObjectMapper: ObjectMapper = objectMapper, + shallRetryRequestsClientBasePredicate: Predicate> = Predicate { it is Err }, + log: Logger = LogManager.getLogger(VertxHttpJsonRpcClient::class.java), + requestResponseLogLevel: Level = this.requestResponseLogLevel, + failuresLogLevel: Level = this.failuresLogLevel + ): JsonRpcV2Client { + assert(endpoints.isNotEmpty()) { "endpoints set is empty " } + // create base client + return if (maxInflightRequestsPerClient != null || endpoints.size > 1) { + createWithLoadBalancing( + endpoints = endpoints, + maxInflightRequestsPerClient = maxInflightRequestsPerClient!!, + httpVersion = httpVersion, + requestObjectMapper = requestObjectMapper, + responseObjectMapper = responseObjectMapper, + log = log, + requestResponseLogLevel = requestResponseLogLevel, + failuresLogLevel = failuresLogLevel + ) + } else { + create( + endpoint = endpoints.first(), + httpVersion = httpVersion, + requestObjectMapper = requestObjectMapper, + responseObjectMapper = responseObjectMapper, + log = log, + requestResponseLogLevel = requestResponseLogLevel, + failuresLogLevel = failuresLogLevel + ) + }.let { + // Wrap the client with a retryer + JsonRpcRequestRetryerV2( + vertx = vertx, + delegate = it, + requestRetry = retryConfig, + requestObjectMapper = requestObjectMapper, + shallRetryRequestsClientBasePredicate = shallRetryRequestsClientBasePredicate, + failuresLogLevel = failuresLogLevel, + log = log + ) + }.let { + // Wrap the client with a v2 client helper + JsonRpcV2ClientImpl( + delegate = it, + idSupplier = requestIdSupplier + ) + } + } } diff --git a/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImplTest.kt b/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImplTest.kt new file mode 100644 index 000000000..c6a008993 --- /dev/null +++ b/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/JsonRpcV2ClientImplTest.kt @@ -0,0 +1,672 @@ +package net.consensys.linea.jsonrpc.client + +import build.linea.s11n.jackson.BigIntegerToHexSerializer +import build.linea.s11n.jackson.ByteArrayToHexSerializer +import build.linea.s11n.jackson.JIntegerToHexSerializer +import build.linea.s11n.jackson.ULongToHexSerializer +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.module.SimpleModule +import com.fasterxml.jackson.databind.node.ArrayNode +import com.fasterxml.jackson.databind.node.ObjectNode +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.github.michaelbull.result.Ok +import com.github.michaelbull.result.Result +import com.github.michaelbull.result.getOr +import com.github.michaelbull.result.orElseThrow +import com.github.tomakehurst.wiremock.WireMockServer +import com.github.tomakehurst.wiremock.client.WireMock.containing +import com.github.tomakehurst.wiremock.client.WireMock.post +import com.github.tomakehurst.wiremock.client.WireMock.status +import com.github.tomakehurst.wiremock.core.WireMockConfiguration +import com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED +import io.micrometer.core.instrument.simple.SimpleMeterRegistry +import io.vertx.core.Vertx +import io.vertx.core.json.JsonObject +import io.vertx.junit5.VertxExtension +import net.consensys.decodeHex +import net.consensys.linea.jsonrpc.JsonRpcErrorResponseException +import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import tech.pegasys.teku.infrastructure.async.SafeFuture +import java.math.BigInteger +import java.net.ConnectException +import java.net.URI +import java.net.URL +import java.util.concurrent.ExecutionException +import java.util.function.Predicate +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds +import kotlin.time.toJavaDuration + +@ExtendWith(VertxExtension::class) +class JsonRpcV2ClientImplTest { + private lateinit var vertx: Vertx + private lateinit var factory: VertxHttpJsonRpcClientFactory + private lateinit var client: JsonRpcV2Client + private lateinit var wiremock: WireMockServer + private val path = "/api/v1?appKey=1234" + private lateinit var meterRegistry: SimpleMeterRegistry + private lateinit var endpoint: URL + private val defaultRetryConfig = retryConfig(maxRetries = 2u, timeout = 8.seconds, backoffDelay = 5.milliseconds) + + private val defaultObjectMapper = jacksonObjectMapper() + private val objectMapperBytesAsHex = jacksonObjectMapper() + .registerModules( + SimpleModule().apply { + this.addSerializer(ByteArray::class.java, ByteArrayToHexSerializer) + this.addSerializer(ULong::class.java, ULongToHexSerializer) + } + ) + private val jsonRpcResultOk = """{"jsonrpc": "2.0", "id": 1, "result": "OK"}""" + + private fun retryConfig( + maxRetries: UInt = 2u, + timeout: Duration = 8.seconds, // bellow 2s we may have flacky tests when running whole test suite in parallel + backoffDelay: Duration = 5.milliseconds + ) = RequestRetryConfig( + maxRetries = maxRetries, + timeout = timeout, + backoffDelay = backoffDelay + ) + + private fun createClientAndSetupWireMockServer( + vertx: Vertx, + responseObjectMapper: ObjectMapper = defaultObjectMapper, + requestObjectMapper: ObjectMapper = defaultObjectMapper, + retryConfig: RequestRetryConfig = defaultRetryConfig, + shallRetryRequestsClientBasePredicate: Predicate> = Predicate { false } + ): JsonRpcV2Client { + wiremock = WireMockServer(WireMockConfiguration.options().dynamicPort()) + wiremock.start() + endpoint = URI(wiremock.baseUrl() + path).toURL() + + return factory.createV2( + vertx = vertx, + endpoints = setOf(endpoint), + retryConfig = retryConfig, + requestObjectMapper = requestObjectMapper, + responseObjectMapper = responseObjectMapper, + shallRetryRequestsClientBasePredicate = shallRetryRequestsClientBasePredicate + ) + } + + @BeforeEach + fun beforeEach(vertx: Vertx) { + this.vertx = vertx + this.meterRegistry = SimpleMeterRegistry() + this.factory = VertxHttpJsonRpcClientFactory(vertx, meterRegistry) + this.client = createClientAndSetupWireMockServer(vertx) + } + + @AfterEach + fun tearDown() { + wiremock.stop() + } + + private fun WireMockServer.jsonRequest(requestIndex: Int = 0): String { + return this.serveEvents.serveEvents[requestIndex].request.bodyAsString + } + + @Test + fun `when request is a list of params shall serialize to json array`() { + replyRequestWith(200, jsonRpcResultOk) + + client.makeRequest( + method = "someMethod", + params = listOf("superUser", "Alice"), + resultMapper = { it } + ).get() + + assertThatJson(wiremock.jsonRequest()).isEqualTo( + """ + { + "jsonrpc": "2.0", + "id":"${'$'}{json-unit.any-number}", + "method": "someMethod", + "params": ["superUser", "Alice"] + } + """ + ) + } + + @Test + fun `when request is a map of params shall serialize to json object`() { + replyRequestWith(200, jsonRpcResultOk) + + client.makeRequest( + method = "someMethod", + params = mapOf("superUser" to "Alice"), + resultMapper = { it } + ).get() + + assertThatJson(wiremock.jsonRequest()).isEqualTo( + """ + { + "jsonrpc": "2.0", + "id":"${'$'}{json-unit.any-number}", + "method": "someMethod", + "params": {"superUser":"Alice"} + } + """ + ) + } + + private data class User( + val name: String, + val email: String, + val address: ByteArray, + val value: ULong + ) + + @Test + fun `when request is Pojo object shall serialize to json object`() { + replyRequestWith(200, jsonRpcResultOk) + + client.makeRequest( + method = "someMethod", + params = User(name = "John", email = "email@example.com", address = "0x01ffbb".decodeHex(), value = 987UL), + resultMapper = { it } + ).get() + // 0x01ffbb -> "Af+7" in Base64, jackon's default encoding for ByteArray + assertThatJson(wiremock.jsonRequest()).isEqualTo( + """ + { + "jsonrpc": "2.0", + "id":"${'$'}{json-unit.any-number}", + "method": "someMethod", + "params": {"name":"John", "email":"email@example.com", "address":"Af+7", "value":987} + } + """ + ) + } + + @Test + fun `request params shall use defined objectMapper and not affect json-rpc envelope`() { + val obj = User(name = "John", email = "email@example.com", address = "0x01ffbb".decodeHex(), value = 987UL) + + createClientAndSetupWireMockServer(vertx, requestObjectMapper = defaultObjectMapper).also { client -> + replyRequestWith(200, jsonRpcResultOk) + client.makeRequest( + method = "someMethod", + params = obj, + resultMapper = { it } + ).get() + + assertThatJson(wiremock.jsonRequest()).isEqualTo( + """ + { + "jsonrpc": "2.0", + "id":"${'$'}{json-unit.any-number}", + "method": "someMethod", + "params": {"name":"John", "email":"email@example.com", "address":"Af+7", "value":987} + } + """ + ) + wiremock.stop() + } + + val objMapperWithNumbersAsHex = jacksonObjectMapper() + .registerModules( + SimpleModule().apply { + this.addSerializer(ByteArray::class.java, ByteArrayToHexSerializer) + this.addSerializer(ULong::class.java, ULongToHexSerializer) + this.addSerializer(Integer::class.java, JIntegerToHexSerializer) + this.addSerializer(BigInteger::class.java, BigIntegerToHexSerializer) + } + ) + + createClientAndSetupWireMockServer(vertx, requestObjectMapper = objMapperWithNumbersAsHex).also { client -> + replyRequestWith(200, jsonRpcResultOk) + client.makeRequest( + method = "someMethod", + params = obj, + resultMapper = { it } + ).get() + + assertThatJson(wiremock.jsonRequest()).isEqualTo( + """ + { + "jsonrpc": "2.0", + "id":"${'$'}{json-unit.any-number}", + "method": "someMethod", + "params": {"name":"John", "email":"email@example.com", "address":"0x01ffbb", "value": "0x3db"} + } + """ + ) + } + } + + @Test + fun `when multiple requests are made, each request shall have an unique id`() { + replyRequestWith(200, jsonRpcResultOk) + val numberOfRequests = 20 + val requestsPromises = IntRange(1, numberOfRequests).map { index -> + client.makeRequest( + method = "someMethod", + params = listOf(index), + resultMapper = { it } + ) + } + SafeFuture.collectAll(requestsPromises.stream()).get() + assertThat(wiremock.serveEvents.serveEvents.size).isEqualTo(numberOfRequests) + + val ids = wiremock.serveEvents.serveEvents.fold(mutableSetOf()) { acc, event -> + val index = JsonObject(event.request.bodyAsString).getString("id") + acc.add(index) + acc + } + assertThat(ids.size).isEqualTo(numberOfRequests) + } + + @Test + fun `when result is null resolves with null`() { + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": null}""") + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + assertThat(response).isNull() + } + } + + @Test + fun `when result is string resolves with String`() { + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": "hello :)"}""") + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + assertThat(response).isEqualTo("hello :)") + } + } + + @Test + fun `when result is Int Number resolves with Int`() { + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": 42}""") + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + assertThat(response).isEqualTo(42) + } + } + + @Test + fun `when result is Long Number resolves with Long`() { + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": ${Long.MAX_VALUE}}""") + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + assertThat(response).isEqualTo(Long.MAX_VALUE) + } + } + + @Test + fun `when result is Floating point Number resolves with Double`() { + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": 3.14}""") + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + assertThat(response).isEqualTo(3.14) + } + } + + @Test + fun `when result is an Object, returns JsonNode`() { + replyRequestWith( + 200, + """{"jsonrpc": "2.0", "id": 1, "result": {"name": "Alice", "age": 23}}""" + ) + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + val expectedObj: ObjectNode = + objectMapperBytesAsHex.readTree("""{"name": "Alice", "age": 23}""") as ObjectNode + assertThat(response).isEqualTo(expectedObj) + } + } + + @Test + fun `when result is an Array, return JsonNode`() { + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": [1, 2, 3]}""") + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + .get() + .also { response -> + val expectedArray: ArrayNode = objectMapperBytesAsHex.readTree("[1, 2, 3]") as ArrayNode + assertThat(response).isEqualTo(expectedArray) + } + } + + @Test + fun `when result transforms result shall return it`() { + data class SimpleUser(val name: String, val age: Int) + replyRequestWith(200, """{"jsonrpc": "2.0", "id": 1, "result": {"name": "Alice", "age": 23}}""") + + val expectedUser = SimpleUser("Alice", 23) + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { + it as JsonNode + SimpleUser(it.get("name").asText(), it.get("age").asInt()) + } + ) + .get() + .also { response -> + assertThat(response).isEqualTo(expectedUser) + } + + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { + it as JsonNode + defaultObjectMapper.treeToValue(it, SimpleUser::class.java) + } + ) + .get() + .also { response -> + assertThat(response).isEqualTo(expectedUser) + } + } + + @Test + fun `when it gets a json-prc error rejects with JsonRpcErrorException cause with error code and message`() { + replyRequestWith( + 200, + """{ + "jsonrpc": "2.0", + "id": 1, + "error": { + "code": -32602, + "message": "Invalid params", + "data": { + "field1": {"key1": "value1", "key2": 20, "key3": [1, 2, 3], "key4": null} + } + } + } + """.trimMargin() + ) + + assertThat( + client.makeRequest( + method = "someMethod", + params = emptyList(), + resultMapper = { it } + ) + ).failsWithin(10.seconds.toJavaDuration()) + .withThrowableThat() + .isInstanceOfSatisfying(ExecutionException::class.java) { + assertThat(it.cause).isInstanceOf(JsonRpcErrorResponseException::class.java) + val cause = it.cause as JsonRpcErrorResponseException + assertThat(cause.rpcErrorCode).isEqualTo(-32602) + assertThat(cause.rpcErrorMessage).isEqualTo("Invalid params") + val expectedData = mapOf( + "field1" to mapOf( + "key1" to "value1", + "key2" to 20, + "key3" to listOf(1, 2, 3), + "key4" to null + ) + ) + assertThat(cause.rpcErrorData).isEqualTo(expectedData) + } + } + + @Test + fun `when it gets an error propagates to shallRetryRequestPredicate and retries while is true`() { + createClientAndSetupWireMockServer( + vertx, + retryConfig = retryConfig(maxRetries = 10u) + ).also { client -> + val responses = listOf( + 500 to "Internal Error", + 200 to "Invalid Json", + 200 to """{"jsonrpc": "2.0", "id": 1, "error": {"code": -32602, "message": "Invalid params"}}""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": null }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "some result" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "expected result" }""" + ) + replyRequestsWith(responses = responses) + val retryPredicateCalls = mutableListOf>() + + client.makeRequest( + method = "someMethod", + params = emptyList(), + shallRetryRequestPredicate = { + retryPredicateCalls.add(it) + it != Ok("EXPECTED RESULT") + }, + resultMapper = { + it as String? + it?.uppercase() + } + ).get() + + assertThat(wiremock.serveEvents.serveEvents).hasSize(responses.size) + assertThat(retryPredicateCalls).hasSize(responses.size) + assertThatThrownBy { retryPredicateCalls[0].orElseThrow() } + .isInstanceOfSatisfying(Exception::class.java) { + assertThat(it.message).contains("HTTP errorCode=500, message=Server Error") + } + assertThatThrownBy { retryPredicateCalls[1].orElseThrow() } + .isInstanceOfSatisfying(IllegalArgumentException::class.java) { + assertThat(it.message).contains("Error parsing JSON-RPC response") + } + assertThatThrownBy { retryPredicateCalls[2].orElseThrow() } + .isInstanceOfSatisfying(JsonRpcErrorResponseException::class.java) { + assertThat(it.rpcErrorCode).isEqualTo(-32602) + assertThat(it.rpcErrorMessage).isEqualTo("Invalid params") + } + assertThat(retryPredicateCalls[3]).isEqualTo(Ok(value = null)) + assertThat(retryPredicateCalls[4]).isEqualTo(Ok(value = "SOME RESULT")) + assertThat(retryPredicateCalls[5]).isEqualTo(Ok(value = "EXPECTED RESULT")) + } + } + + @Test + fun `when it has connection error propagates to shallRetryRequestPredicate and retries while is true`() { + createClientAndSetupWireMockServer( + vertx, + retryConfig = retryConfig(maxRetries = 10u) + ).also { client -> + // stop the server to simulate connection error + wiremock.stop() + + val retryPredicateCalls = mutableListOf>() + + val reqFuture = client.makeRequest( + method = "someMethod", + params = emptyList(), + shallRetryRequestPredicate = { + retryPredicateCalls.add(it) + retryPredicateCalls.size < 2 + }, + resultMapper = { it as String? } + ) + + assertThatThrownBy { reqFuture.get() } + .isInstanceOfSatisfying(ExecutionException::class.java) { + assertThat(it.cause).isInstanceOfSatisfying(ConnectException::class.java) { + assertThat(it.message).contains("Connection refused: localhost/127.0.0.1:") + } + } + + assertThat(retryPredicateCalls.size).isEqualTo(2) + assertThatThrownBy { retryPredicateCalls[0].orElseThrow() } + .isInstanceOfSatisfying(ConnectException::class.java) { + assertThat(it.message).contains("Connection refused: localhost/127.0.0.1:") + } + } + } + + @Test + fun `when it has connection error propagates to shallRetryRequestPredicate and retries until retry config elapses`() { + createClientAndSetupWireMockServer( + vertx, + retryConfig = retryConfig(maxRetries = 2u, timeout = 8.seconds, backoffDelay = 5.milliseconds) + ).also { client -> + // stop the server to simulate connection error + wiremock.stop() + + val retryPredicateCalls = mutableListOf>() + + val reqFuture = client.makeRequest( + method = "someMethod", + params = emptyList(), + shallRetryRequestPredicate = { + retryPredicateCalls.add(it) + true // keep retrying + }, + resultMapper = { it as String? } + ) + + assertThatThrownBy { reqFuture.get() } + .isInstanceOfSatisfying(ExecutionException::class.java) { + assertThat(it.cause).isInstanceOfSatisfying(ConnectException::class.java) { + assertThat(it.message).contains("Connection refused: localhost/127.0.0.1:") + } + } + assertThat(retryPredicateCalls).hasSizeBetween(1, 3) + } + } + + @Test + fun `when shared predicate is defined shall retry when any of them returns true`() { + val baseRetryPredicateCalls = mutableListOf>() + val baseRetryPredicate = Predicate> { + baseRetryPredicateCalls.add(it) + it as Ok + (it.value as String).startsWith("retry_a") + } + createClientAndSetupWireMockServer( + vertx, + retryConfig = RequestRetryConfig( + maxRetries = 10u, + timeout = 5.minutes, + backoffDelay = 1.milliseconds + ), + shallRetryRequestsClientBasePredicate = baseRetryPredicate + ).also { client -> + replyRequestsWith( + listOf( + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "retry_a_0" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "retry_a_1" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "retry_a_2" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "retry_b_3" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "retry_b_4" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "retry_b_5" }""", + 200 to """{"jsonrpc": "2.0", "id": 1, "result": "some_result" }""" + ) + ) + val retryPredicateCalls = mutableListOf>() + val reqFuture = client.makeRequest( + method = "someMethod", + params = emptyList(), + shallRetryRequestPredicate = { + retryPredicateCalls.add(it) + it.getOr("").startsWith("retry_b") + }, + resultMapper = { it as String } + ) + + assertThat(reqFuture.get()).isEqualTo("some_result") + assertThat(baseRetryPredicateCalls).hasSize(7) + // this extra assertion is not necessary for correctness. + // however, if it breaks raises awareness that 2nd predicate may be evaluated without need + assertThat(retryPredicateCalls.map { it.getOr(-1) }).isEqualTo( + listOf( + "retry_b_3", + "retry_b_4", + "retry_b_5", + "some_result" + ) + ) + } + } + + private fun replyRequestWith(statusCode: Int, body: String?) { + wiremock.stubFor( + post(path) + .withHeader("Content-Type", containing("application/json")) + .willReturn( + status(statusCode) + .withHeader("Content-type", "text/plain") + .apply { if (body != null) withBody(body) } + ) + ) + } + + private fun replyRequestsWith(responses: List>) { + val (firstResponseStatus, firstResponseBody) = responses.first() + wiremock.stubFor( + post(path) + .withHeader("Content-Type", containing("application/json")) + .inScenario("retry") + .whenScenarioStateIs(STARTED) + .willReturn( + status(firstResponseStatus) + .withHeader("Content-type", "text/plain") + .apply { if (firstResponseBody != null) withBody(firstResponseBody) } + ) + .willSetStateTo("req_0") + ) + + responses + .drop(1) + .forEachIndexed { index, (statusCode, body) -> + wiremock.stubFor( + post(path) + .withHeader("Content-Type", containing("application/json")) + .inScenario("retry") + .whenScenarioStateIs("req_$index") + .willReturn( + status(statusCode) + .withHeader("Content-type", "text/plain") + .apply { if (body != null) withBody(body) } + ) + .willSetStateTo("req_${index + 1}") + ) + } + } +} diff --git a/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientTest.kt b/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientTest.kt index c080675a1..b7506bfbd 100644 --- a/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientTest.kt +++ b/jvm-libs/json-rpc/src/test/kotlin/net/consensys/linea/jsonrpc/client/VertxHttpJsonRpcClientTest.kt @@ -16,6 +16,7 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry import io.vertx.core.Vertx import io.vertx.core.http.HttpClientOptions import io.vertx.core.http.HttpVersion +import io.vertx.core.json.JsonArray import io.vertx.core.json.JsonObject import net.consensys.linea.async.get import net.consensys.linea.async.toSafeFuture @@ -115,27 +116,37 @@ class VertxHttpJsonRpcClientTest { } @Test - fun makesRequest_successNullResult() { + fun makesRequest_success_result_is_null() { replyRequestWith(JsonObject().put("jsonrpc", "2.0").put("id", "1").put("result", null)) - val response = - client.makeRequest(JsonRpcRequestListParams("2.0", 1, "eth_blockNumber", emptyList())).get() - - assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", null))) + client.makeRequest(JsonRpcRequestListParams("2.0", 1, "eth_blockNumber", emptyList())).get() + .also { response -> + assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", null))) + } } @Test - fun makesRequest_successSingleValue() { + fun makesRequest_success_result_is_number() { replyRequestWith(JsonObject().put("jsonrpc", "2.0").put("id", "1").put("result", 3)) - val response = - client.makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumber", emptyList())).get() + client.makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumber", emptyList())).get() + .also { response -> + assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", 3))) + } + } - assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", 3))) + @Test + fun makesRequest_success_result_is_string() { + replyRequestWith(JsonObject().put("jsonrpc", "2.0").put("id", "1").put("result", "0x1234")) + + client.makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumber", emptyList())).get() + .also { response -> + assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", "0x1234"))) + } } @Test - fun makesRequest_successJsonObject() { + fun makesRequest_success_result_is_Object() { replyRequestWith( JsonObject() .put("jsonrpc", "2.0") @@ -143,11 +154,60 @@ class VertxHttpJsonRpcClientTest { .put("result", JsonObject().put("odd", 23).put("even", 10)) ) - val response = - client.makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList())).get() + client + .makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList())) + .get() + .also { response -> + val expectedJsonNode = JsonObject("""{"odd":23,"even":10}""") + assertThat(response) + .isEqualTo(Ok(JsonRpcSuccessResponse("1", expectedJsonNode))) + } + + client + .makeRequest( + request = JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList()), + resultMapper = ::toPrimitiveOrJacksonJsonNode + ) + .get() + .also { response -> + val expectedJsonNode = objectMapper.readTree("""{"odd":23,"even":10}""") + assertThat(response) + .isEqualTo(Ok(JsonRpcSuccessResponse("1", expectedJsonNode))) + } + } - assertThat(response) - .isEqualTo(Ok(JsonRpcSuccessResponse("1", JsonObject().put("odd", 23).put("even", 10)))) + @Test + fun makesRequest_success_result_is_array() { + replyRequestWith( + statusCode = 200, + """{ + |"jsonrpc": "2.0", + |"id": "1", + |"result": ["a", 2, "c", 4] + |} + """.trimMargin() + ) + + client + .makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList())) + .get() + .also { response -> + val expectedJsonNode = JsonArray("""["a", 2, "c", 4]""") + assertThat(response) + .isEqualTo(Ok(JsonRpcSuccessResponse("1", expectedJsonNode))) + } + + client + .makeRequest( + request = JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList()), + resultMapper = ::toPrimitiveOrJacksonJsonNode + ) + .get() + .also { response -> + val expectedJsonNode = objectMapper.readTree("""["a", 2, "c", 4]""") + assertThat(response) + .isEqualTo(Ok(JsonRpcSuccessResponse("1", expectedJsonNode))) + } } @Test @@ -156,12 +216,13 @@ class VertxHttpJsonRpcClientTest { JsonObject().put("jsonrpc", "2.0").put("id", "1").put("result", "some_random_value") ) val resultMapper = { value: Any? -> (value as String).uppercase() } - val response = - client - .makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList()), resultMapper) - .get() - assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", "SOME_RANDOM_VALUE"))) + client + .makeRequest(JsonRpcRequestListParams("2.0", 1, "randomNumbers", emptyList()), resultMapper) + .get() + .also { response -> + assertThat(response).isEqualTo(Ok(JsonRpcSuccessResponse("1", "SOME_RANDOM_VALUE"))) + } } @Test diff --git a/traces-api-facade/conflation/src/main/kotlin/net/consensys/linea/traces/RawJsonTracesConflator.kt b/traces-api-facade/conflation/src/main/kotlin/net/consensys/linea/traces/RawJsonTracesConflator.kt index 1d553c424..c6bfc7e79 100644 --- a/traces-api-facade/conflation/src/main/kotlin/net/consensys/linea/traces/RawJsonTracesConflator.kt +++ b/traces-api-facade/conflation/src/main/kotlin/net/consensys/linea/traces/RawJsonTracesConflator.kt @@ -1,5 +1,7 @@ package net.consensys.linea.traces +import com.fasterxml.jackson.databind.MapperFeature +import com.fasterxml.jackson.databind.json.JsonMapper import com.github.michaelbull.result.Err import com.github.michaelbull.result.Ok import com.github.michaelbull.result.Result @@ -1028,22 +1030,18 @@ class ConflatedTrace : ConflatedTraceStorage() { fun reAssembleRom() { fun hiLoToAddr(hi: String, lo: String): BigInteger { val addrShift = 256.toBigInteger().pow(16) - return hi - .toBigInteger() - .multiply(addrShift) - .add(lo.toBigInteger()) + return hi.toBigInteger().multiply(addrShift).add(lo.toBigInteger()) } val idxs = (0 until this.rom.PC.size).toList() - val sortedIdxs = - idxs.sortedWith( - compareBy( - { hiLoToAddr(this.rom.SC_ADDRESS_HI[it], this.rom.SC_ADDRESS_LO[it]) }, - // we want the initcode *FIRST* - { -this.rom.IS_INITCODE[it].toInt() }, - { this.rom.PC[it].toInt() } - ) + val sortedIdxs = idxs.sortedWith( + compareBy( + { hiLoToAddr(this.rom.SC_ADDRESS_HI[it], this.rom.SC_ADDRESS_LO[it]) }, + // we want the initcode *FIRST* + { -this.rom.IS_INITCODE[it].toInt() }, + { this.rom.PC[it].toInt() } ) + ) // Awfully suboptimal, but it just works val copiedRom = this.rom.copy() @@ -1054,9 +1052,8 @@ class ConflatedTrace : ConflatedTraceStorage() { if (i > 0) { for (column in Rom::class.memberProperties) { if (column != Rom::ADDRESS_INDEX && column != Rom::CODE_FRAGMENT_INDEX) { - duplicate = duplicate && - getColumn(copiedRom, column)[sortedIdxs[i]].toBigInteger() - .equals(getColumn(this.rom, column).last().toBigInteger()) + duplicate = duplicate && getColumn(copiedRom, column)[sortedIdxs[i]].toBigInteger() + .equals(getColumn(this.rom, column).last().toBigInteger()) } } } else { @@ -1163,7 +1160,9 @@ class ConflatedTrace : ConflatedTraceStorage() { } } -class RawJsonTracesConflator(val tracesEngineVersion: String) : TracesConflator { +class RawJsonTracesConflator(private val tracesEngineVersion: String) : TracesConflator { + private val objectMapper: JsonMapper = JsonMapper.builder().disable(MapperFeature.USE_GETTERS_AS_SETTERS).build() + private val log: Logger = LogManager.getLogger(this::class.java) override fun conflateTraces( @@ -1181,12 +1180,11 @@ class RawJsonTracesConflator(val tracesEngineVersion: String) : TracesConflator log.trace("Parsing trace: {}", jsonPath) trace.getTrace(jsonPath)?.let { if (!it.isEmpty) { - ax.add(it.mapTo(klass)) + ax.add(objectMapper.convertValue(it, klass)) } + } ?: run { + log.warn("Could not parse object with path: '{}'", jsonPath.joinToString(".")) } - ?: run { - log.warn("Could not parse object with path: '{}'", jsonPath.joinToString(".")) - } } } From 086e0ad077b9681c33b32381ab73eb2e7291b836 Mon Sep 17 00:00:00 2001 From: The Dark Jester Date: Fri, 18 Oct 2024 10:18:42 -0700 Subject: [PATCH 4/6] Use NatSpec for ITokenBridge events and errors (#202) * Use NatSpec for ITokenBridge events and errors * pass 1 of NatSpec cleanup * natspec pass 2 * correct wording on NatSpec * use correct indexed keyword location --- contracts/contracts/ZkEvmV2.sol | 7 +- .../contracts/interfaces/IPauseManager.sol | 1 + .../interfaces/l1/IL1MessageManager.sol | 1 + .../interfaces/l1/IL1MessageManagerV1.sol | 2 +- .../contracts/interfaces/l1/ILineaRollup.sol | 16 +- .../interfaces/l2/IL2MessageManager.sol | 5 +- .../interfaces/l2/IL2MessageServiceV1.sol | 1 + .../lib/L2MessageServicePauseManager.sol | 2 +- .../contracts/lib/LineaRollupPauseManager.sol | 2 +- .../contracts/lib/TokenBridgePauseManager.sol | 2 +- .../messageService/MessageServiceBase.sol | 2 +- .../l2/v1/L2MessageServiceV1.sol | 1 + .../messageService/lib/MessageHashing.sol | 6 +- .../tokenBridge/CustomBridgedToken.sol | 2 +- .../tokenBridge/interfaces/ITokenBridge.sol | 142 +++++++++++++++++- .../tokenBridge/lib/StorageFiller39.sol | 7 +- 16 files changed, 172 insertions(+), 27 deletions(-) diff --git a/contracts/contracts/ZkEvmV2.sol b/contracts/contracts/ZkEvmV2.sol index e1ea16de3..f5449c84c 100644 --- a/contracts/contracts/ZkEvmV2.sol +++ b/contracts/contracts/ZkEvmV2.sol @@ -7,7 +7,7 @@ import { L1MessageServiceV1 } from "./messageService/l1/v1/L1MessageServiceV1.so import { IZkEvmV2 } from "./interfaces/l1/IZkEvmV2.sol"; import { IPlonkVerifier } from "./interfaces/l1/IPlonkVerifier.sol"; /** - * @title Contract to manage cross-chain messaging on L1 and rollup proving. + * @title Contract to manage cross-chain L1 rollup proving. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ @@ -53,12 +53,11 @@ abstract contract ZkEvmV2 is Initializable, AccessControlUpgradeable, L1MessageS assembly { let dataOffset := add(result, 0x20) - // Store the modified first 32 bytes back into memory overwriting the location after having swapped out the selector + // Store the modified first 32 bytes back into memory overwriting the location after having swapped out the selector. mstore( dataOffset, or( - // InvalidProofOrProofVerificationRanOutOfGas(string) = 0xca389c44bf373a5a506ab5a7d8a53cb0ea12ba7c5872fd2bc4a0e31614c00a85 - // Using the selector from a bytes4 variable and shl results in 0x00000000 + // InvalidProofOrProofVerificationRanOutOfGas(string) = 0xca389c44bf373a5a506ab5a7d8a53cb0ea12ba7c5872fd2bc4a0e31614c00a85. shl(224, 0xca389c44), and(mload(dataOffset), 0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff) ) diff --git a/contracts/contracts/interfaces/IPauseManager.sol b/contracts/contracts/interfaces/IPauseManager.sol index c3525080e..0b00b9a98 100644 --- a/contracts/contracts/interfaces/IPauseManager.sol +++ b/contracts/contracts/interfaces/IPauseManager.sol @@ -22,6 +22,7 @@ interface IPauseManager { * @notice Enum defining the different types of pausing. * @dev The pause types are used to pause and unpause specific functionality. * @dev The UNUSED pause type is used as a default value to avoid accidental general pausing. + * @dev Enums are uint8 by default. */ enum PauseType { UNUSED, diff --git a/contracts/contracts/interfaces/l1/IL1MessageManager.sol b/contracts/contracts/interfaces/l1/IL1MessageManager.sol index be24514e9..377318da5 100644 --- a/contracts/contracts/interfaces/l1/IL1MessageManager.sol +++ b/contracts/contracts/interfaces/l1/IL1MessageManager.sol @@ -48,6 +48,7 @@ interface IL1MessageManager { /** * @notice Checks if the L2->L1 message is claimed or not. * @param _messageNumber The message number on L2. + * @return Bool indicating claimed or not. */ function isMessageClaimed(uint256 _messageNumber) external view returns (bool); } diff --git a/contracts/contracts/interfaces/l1/IL1MessageManagerV1.sol b/contracts/contracts/interfaces/l1/IL1MessageManagerV1.sol index 4a1296476..ff35c77cc 100644 --- a/contracts/contracts/interfaces/l1/IL1MessageManagerV1.sol +++ b/contracts/contracts/interfaces/l1/IL1MessageManagerV1.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.26; /** - * @title L1 Message manager V1 interface for pre-existing functions, events and errors. + * @title L1 Message manager V1 interface for pre-existing errors. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ diff --git a/contracts/contracts/interfaces/l1/ILineaRollup.sol b/contracts/contracts/interfaces/l1/ILineaRollup.sol index 2d434eb96..9f93133bd 100644 --- a/contracts/contracts/interfaces/l1/ILineaRollup.sol +++ b/contracts/contracts/interfaces/l1/ILineaRollup.sol @@ -12,9 +12,9 @@ import { IPermissionsManager } from "../../interfaces/IPermissionsManager.sol"; interface ILineaRollup { /** * @notice Initialization data structure for the LineaRollup contract. - * @param initialStateRootHash The initial state root hash at migration used for proof verification. - * @param initialL2BlockNumber The initial block number at migration. - * @param genesisTimestamp The L2 genesis timestamp for first finalization. + * @param initialStateRootHash The initial state root hash at initialization used for proof verification. + * @param initialL2BlockNumber The initial block number at initialization. + * @param genesisTimestamp The L2 genesis timestamp for first initialization. * @param defaultVerifier The default verifier for rollup proofs. * @param rateLimitPeriodInSeconds The period in which withdrawal amounts and fees will be accumulated. * @param rateLimitAmountInWei The limit allowed for withdrawing in the rate limit period. @@ -40,11 +40,11 @@ interface ILineaRollup { /** * @notice Supporting data for compressed calldata submission including compressed data. - * @dev finalStateRootHash is used to set state root at the end of the data. + * @dev finalStateRootHash is used to set state root hash at the end of the data. * @dev firstBlockInData is the first block that is included in the data submitted. * @dev finalBlockInData is the last block that is included in the data submitted. * @dev snarkHash is the computed hash for compressed data (using a SNARK-friendly hash function) that aggregates per data submission to be used in public input. - * @dev compressedData is the compressed transaction data. It contains ordered data for each L2 block - l2Timestamps, the encoded txData. + * @dev compressedData is the compressed transaction data. It contains ordered data for each L2 block - l2Timestamps, the encoded transaction data. */ struct SubmissionDataV2 { bytes32 finalStateRootHash; @@ -190,9 +190,9 @@ interface ILineaRollup { * @notice Emitted when L2 blocks have been finalized on L1. * @param startBlockNumber The indexed L2 block number indicating which block the finalization the data starts from. * @param endBlockNumber The indexed L2 block number indicating which block the finalization the data ends on. - * @param shnarf The shnarf being set as currentFinalizedShnarf in the current finalization. - * @param parentStateRootHash The indexed parent L2 state root hash that the current finalization starts from. - * @param finalStateRootHash The indexed L2 state root hash that the current finalization ends on. + * @param shnarf The indexed shnarf being set as currentFinalizedShnarf in the current finalization. + * @param parentStateRootHash The parent L2 state root hash that the current finalization starts from. + * @param finalStateRootHash The L2 state root hash that the current finalization ends on. */ event DataFinalizedV3( uint256 indexed startBlockNumber, diff --git a/contracts/contracts/interfaces/l2/IL2MessageManager.sol b/contracts/contracts/interfaces/l2/IL2MessageManager.sol index db3cdee65..17035e29a 100644 --- a/contracts/contracts/interfaces/l2/IL2MessageManager.sol +++ b/contracts/contracts/interfaces/l2/IL2MessageManager.sol @@ -9,7 +9,7 @@ pragma solidity 0.8.19; interface IL2MessageManager { /** * @notice Emitted after all messages are anchored on L2 and the latest message index and rolling hash stored. - * @param messageNumber The unique L1 computed indexed message number for the message. + * @param messageNumber The indexed unique L1 computed indexed message number for the message. * @param rollingHash The indexed L1 rolling hash computed for the current message number. * @dev NB: This event is used to provide data to the rollup. The last messageNumber and rollingHash, * emitted in a rollup will be used in the public input for validating the L1->L2 messaging state transition. @@ -18,7 +18,8 @@ interface IL2MessageManager { /** * @dev Emitted when the service switches over to a new version. - * @dev This is currently not in use, but left for future migrations and for existing consumers. + * @dev This is currently not in use, but left for existing consumers. + * @param version The indexed version. */ event ServiceVersionMigrated(uint256 indexed version); diff --git a/contracts/contracts/interfaces/l2/IL2MessageServiceV1.sol b/contracts/contracts/interfaces/l2/IL2MessageServiceV1.sol index 6d902e301..52adf1d0a 100644 --- a/contracts/contracts/interfaces/l2/IL2MessageServiceV1.sol +++ b/contracts/contracts/interfaces/l2/IL2MessageServiceV1.sol @@ -9,6 +9,7 @@ pragma solidity 0.8.19; interface IL2MessageServiceV1 { /** * @notice The Fee Manager sets a minimum fee to address DOS protection. + * @dev MINIMUM_FEE_SETTER_ROLE is required to set the minimum fee. * @param _feeInWei New minimum fee in Wei. */ function setMinimumFee(uint256 _feeInWei) external; diff --git a/contracts/contracts/lib/L2MessageServicePauseManager.sol b/contracts/contracts/lib/L2MessageServicePauseManager.sol index 75e551bd7..6bcbb1283 100644 --- a/contracts/contracts/lib/L2MessageServicePauseManager.sol +++ b/contracts/contracts/lib/L2MessageServicePauseManager.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.19 <=0.8.26; import { PauseManager } from "./PauseManager.sol"; /** - * @title Contract to manage cross-chain function pausing for LineaRollup. + * @title Contract to manage cross-chain function pausing roles for the L2 Message Service. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ diff --git a/contracts/contracts/lib/LineaRollupPauseManager.sol b/contracts/contracts/lib/LineaRollupPauseManager.sol index 484a3c46c..ad21367bd 100644 --- a/contracts/contracts/lib/LineaRollupPauseManager.sol +++ b/contracts/contracts/lib/LineaRollupPauseManager.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.19 <=0.8.26; import { PauseManager } from "./PauseManager.sol"; /** - * @title Contract to manage cross-chain function pausing for LineaRollup. + * @title Contract to manage cross-chain function pausing roles for the LineaRollup. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ diff --git a/contracts/contracts/lib/TokenBridgePauseManager.sol b/contracts/contracts/lib/TokenBridgePauseManager.sol index 90250687d..ec9f4f4fb 100644 --- a/contracts/contracts/lib/TokenBridgePauseManager.sol +++ b/contracts/contracts/lib/TokenBridgePauseManager.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.19 <=0.8.26; import { PauseManager } from "./PauseManager.sol"; /** - * @title Contract to manage cross-chain function pausing for LineaRollup. + * @title Contract to manage cross-chain function pausing roles for the Token Bridge. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ diff --git a/contracts/contracts/messageService/MessageServiceBase.sol b/contracts/contracts/messageService/MessageServiceBase.sol index b40346107..a3ae47674 100644 --- a/contracts/contracts/messageService/MessageServiceBase.sol +++ b/contracts/contracts/messageService/MessageServiceBase.sol @@ -65,7 +65,7 @@ abstract contract MessageServiceBase is Initializable, IGenericErrors { /** * @notice Initializes the message service - * @dev Must be initialized in the initialize function of the main contract or constructor + * @dev Must be initialized in the initialize function of the main contract or constructor. * @param _messageService The message service address, cannot be empty. */ function __MessageServiceBase_init(address _messageService) internal onlyInitializing { diff --git a/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol b/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol index 34f53479f..1ac58b808 100644 --- a/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol +++ b/contracts/contracts/messageService/l2/v1/L2MessageServiceV1.sol @@ -148,6 +148,7 @@ abstract contract L2MessageServiceV1 is /** * @notice The Fee Manager sets a minimum fee to address DOS protection. + * @dev MINIMUM_FEE_SETTER_ROLE is required to set the minimum fee. * @param _feeInWei New minimum fee in Wei. */ function setMinimumFee(uint256 _feeInWei) external onlyRole(MINIMUM_FEE_SETTER_ROLE) { diff --git a/contracts/contracts/messageService/lib/MessageHashing.sol b/contracts/contracts/messageService/lib/MessageHashing.sol index 25034ba1e..90d7ccd0c 100644 --- a/contracts/contracts/messageService/lib/MessageHashing.sol +++ b/contracts/contracts/messageService/lib/MessageHashing.sol @@ -2,14 +2,14 @@ pragma solidity >=0.8.19 <=0.8.26; /** - * @title Library to hash messages. + * @title Library to hash cross-chain messages. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ library MessageHashing { /** * @notice Hashes messages using assembly for efficiency. - * @dev Adding 0x00000000000000000000000000000000000000000000000000000000000000c0 is to indicate the calldata offset. + * @dev Adding 0xc0 is to indicate the calldata offset relative to the memory being added to. * @dev If the calldata is not modulus 32, the extra bit needs to be added on at the end else the hash is wrong. * @param _from The from address. * @param _to The to address. @@ -33,7 +33,7 @@ library MessageHashing { mstore(add(mPtr, 0x40), _fee) mstore(add(mPtr, 0x60), _valueSent) mstore(add(mPtr, 0x80), _messageNumber) - mstore(add(mPtr, 0xa0), 0x00000000000000000000000000000000000000000000000000000000000000c0) + mstore(add(mPtr, 0xa0), 0xc0) mstore(add(mPtr, 0xc0), _calldata.length) let rem := mod(_calldata.length, 0x20) let extra := 0 diff --git a/contracts/contracts/tokenBridge/CustomBridgedToken.sol b/contracts/contracts/tokenBridge/CustomBridgedToken.sol index 552b80f89..029e37657 100644 --- a/contracts/contracts/tokenBridge/CustomBridgedToken.sol +++ b/contracts/contracts/tokenBridge/CustomBridgedToken.sol @@ -5,7 +5,7 @@ import { BridgedToken } from "./BridgedToken.sol"; /** * @title Custom BridgedToken Contract - * @notice Custom ERC20 token manually deployed for the Linea TokenBridge + * @notice Custom ERC20 token manually deployed for the Linea TokenBridge. */ contract CustomBridgedToken is BridgedToken { function initializeV2( diff --git a/contracts/contracts/tokenBridge/interfaces/ITokenBridge.sol b/contracts/contracts/tokenBridge/interfaces/ITokenBridge.sol index ab2b3d56b..50fb356c8 100644 --- a/contracts/contracts/tokenBridge/interfaces/ITokenBridge.sol +++ b/contracts/contracts/tokenBridge/interfaces/ITokenBridge.sol @@ -5,7 +5,7 @@ import { IPauseManager } from "../../interfaces/IPauseManager.sol"; import { IPermissionsManager } from "../../interfaces/IPermissionsManager.sol"; /** - * @title Interface declaring Canonical Token Bridge functions, events and errors. + * @title Interface declaring Canonical Token Bridge struct, functions, events and errors. * @author ConsenSys Software Inc. * @custom:security-contact security-report@linea.build */ @@ -34,48 +34,184 @@ interface ITokenBridge { IPauseManager.PauseTypeRole[] unpauseTypeRoles; } + /** + * @notice Emitted when the token address is reserved. + * @param token The indexed token address. + */ event TokenReserved(address indexed token); + + /** + * @notice Emitted when the token address reservation is removed. + * @param token The indexed token address. + */ event ReservationRemoved(address indexed token); + + /** + * @notice Emitted when the custom token address is set. + * @param nativeToken The indexed nativeToken token address. + * @param customContract The indexed custom contract address. + * @param setBy The indexed address of who set the custom contract. + */ event CustomContractSet(address indexed nativeToken, address indexed customContract, address indexed setBy); - /// @dev DEPRECATED in favor of BridgingInitiatedV2. + + /** + * @notice Emitted when token bridging is initiated. + * @dev DEPRECATED in favor of BridgingInitiatedV2. + * @param sender The indexed sender address. + * @param recipient The recipient address. + * @param token The indexed token address. + * @param amount The indexed token amount. + */ event BridgingInitiated(address indexed sender, address recipient, address indexed token, uint256 indexed amount); + + /** + * @notice Emitted when token bridging is initiated. + * @param sender The indexed sender address. + * @param recipient The indexed recipient address. + * @param token The indexed token address. + * @param amount The token amount. + */ event BridgingInitiatedV2(address indexed sender, address indexed recipient, address indexed token, uint256 amount); - /// @dev DEPRECATED in favor of BridgingFinalizedV2. + + /** + * @notice Emitted when token bridging is finalized. + * @dev DEPRECATED in favor of BridgingFinalizedV2. + * @param nativeToken The indexed native token address. + * @param bridgedToken The indexed bridged token address. + * @param amount The indexed token amount. + * @param recipient The recipient address. + */ event BridgingFinalized( address indexed nativeToken, address indexed bridgedToken, uint256 indexed amount, address recipient ); + + /** + * @notice Emitted when token bridging is finalized. + * @param nativeToken The indexed native token address. + * @param bridgedToken The indexed bridged token address. + * @param amount The token amount. + * @param recipient The indexed recipient address. + */ event BridgingFinalizedV2( address indexed nativeToken, address indexed bridgedToken, uint256 amount, address indexed recipient ); + + /** + * @notice Emitted when a new token is seen being bridged on the origin chain for the first time. + * @param token The indexed token address. + */ event NewToken(address indexed token); + + /** + * @notice Emitted when a new token is deployed. + * @param bridgedToken The indexed bridged token address. + * @param nativeToken The indexed native token address. + */ event NewTokenDeployed(address indexed bridgedToken, address indexed nativeToken); + + /** + * @notice Emitted when the remote token bridge is set. + * @param remoteTokenBridge The indexed remote token bridge address. + * @param setBy The indexed address that set the remote token bridge. + */ event RemoteTokenBridgeSet(address indexed remoteTokenBridge, address indexed setBy); + + /** + * @notice Emitted when the token is set as deployed. + * @dev This can be triggered by anyone calling confirmDeployment on the alternate chain. + * @param token The indexed token address. + */ event TokenDeployed(address indexed token); + + /** + * @notice Emitted when the token deployment is confirmed. + * @dev This can be triggered by anyone provided there is correctly mapped token data. + * @param tokens The token address list. + * @param confirmedBy The indexed address confirming deployment. + */ event DeploymentConfirmed(address[] tokens, address indexed confirmedBy); + + /** + * @notice Emitted when the message service address is set. + * @param newMessageService The indexed new message service address. + * @param oldMessageService The indexed old message service address. + * @param setBy The indexed address setting the new message service address. + */ event MessageServiceUpdated( address indexed newMessageService, address indexed oldMessageService, address indexed setBy ); + /** + * @dev Thrown when attempting to bridge a reserved token. + */ error ReservedToken(address token); + + /** + * @dev Thrown when the remote token bridge is already set. + */ error RemoteTokenBridgeAlreadySet(address remoteTokenBridge); + + /** + * @dev Thrown when attempting to reserve an already bridged token. + */ error AlreadyBridgedToken(address token); + + /** + * @dev Thrown when the permit data is invalid. + */ error InvalidPermitData(bytes4 permitData, bytes4 permitSelector); + + /** + * @dev Thrown when the permit is not from the sender. + */ error PermitNotFromSender(address owner); + + /** + * @dev Thrown when the permit does not grant spending to the bridge. + */ error PermitNotAllowingBridge(address spender); + + /** + * @dev Thrown when the amount being bridged is zero. + */ error ZeroAmountNotAllowed(uint256 amount); + + /** + * @dev Thrown when trying to unreserve a non-reserved token. + */ error NotReserved(address token); + + /** + * @dev Thrown when trying to confirm deployment of a non-deployed token. + */ error TokenNotDeployed(address token); + + /** + * @dev Thrown when trying to set a custom contract on a bridged token. + */ error AlreadyBrigedToNativeTokenSet(address token); + + /** + * @dev Thrown when trying to set a custom contract on an already set token. + */ error NativeToBridgedTokenAlreadySet(address token); + + /** + * @dev Thrown when trying to set a token that is already either native, deployed or reserved. + */ error StatusAddressNotAllowed(address token); + + /** + * @dev Thrown when the decimals for a token cannot be determined. + */ error DecimalsAreUnknown(address token); /** diff --git a/contracts/contracts/tokenBridge/lib/StorageFiller39.sol b/contracts/contracts/tokenBridge/lib/StorageFiller39.sol index ab70c70ad..e2a241b32 100644 --- a/contracts/contracts/tokenBridge/lib/StorageFiller39.sol +++ b/contracts/contracts/tokenBridge/lib/StorageFiller39.sol @@ -1,7 +1,12 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity 0.8.19; -contract StorageFiller39 { +/** + * @title Contract to fill space in storage to maintain storage layout. + * @author ConsenSys Software Inc. + * @custom:security-contact security-report@linea.build + */ +abstract contract StorageFiller39 { /// @dev Keep free storage slots for future implementation updates to avoid storage collision. uint256[39] private __gap_39; } From 017df93188da51430363f158f2f3e294637dee7b Mon Sep 17 00:00:00 2001 From: The Dark Jester Date: Fri, 18 Oct 2024 11:13:09 -0700 Subject: [PATCH 5/6] [Feat] optimize blob submission data (#209) * smart-contract: small tweak on validation and consistent naming on params * use currentL2BlockNumber vs. memory * cache blobFirstBlockNumber * optimize blob number validation * cache snarkhash and finalStateRootHash * remove redundant checks * remove block numbers from blob submits * remove extra space * address naming considerations * use updated ABI for V6 LineaRollup * Use NatSpec for ITokenBridge events and errors (#202) * Use NatSpec for ITokenBridge events and errors * pass 1 of NatSpec cleanup * natspec pass 2 * correct wording on NatSpec * use correct indexed keyword location --------- Signed-off-by: The Dark Jester Co-authored-by: Pedro Novais <1478752+jpnovais@users.noreply.github.com> --- config/common/smart-contract-errors.toml | 3 +- contracts/abi/LineaRollupV6.0.abi | 208 +++------- contracts/contracts/LineaRollup.sol | 162 +++----- .../contracts/interfaces/l1/ILineaRollup.sol | 84 +---- .../test-contracts/TestLineaRollup.sol | 6 +- .../SixInOne/sendBlobTransaction.ts | 19 +- contracts/test/LineaRollup.ts | 356 +++++------------- .../test/common/helpers/dataGeneration.ts | 66 ++-- contracts/test/common/types.ts | 27 +- .../app/config/CoordinatorConfigTest.kt | 3 +- 10 files changed, 273 insertions(+), 661 deletions(-) diff --git a/config/common/smart-contract-errors.toml b/config/common/smart-contract-errors.toml index 36015605a..eaa68c18b 100644 --- a/config/common/smart-contract-errors.toml +++ b/config/common/smart-contract-errors.toml @@ -28,7 +28,7 @@ "b1504a5f" = "BlobSubmissionDataIsMissing" "c0e41e1d" = "EmptyBlobDataAtIndex" "fb4cd6ef" = "FinalBlockDoesNotMatchShnarfFinalBlock" -"2526F108 " = "ShnarfAndFinalBlockNumberLengthsMismatched" +"2526F108" = "ShnarfAndFinalBlockNumberLengthsMismatched" "d3664fb3" = "FinalShnarfWrong" "4e686675" = "L2MerkleRootDoesNotExist" "e5d14425" = "L2MerkleRootAlreadyAnchored" @@ -46,3 +46,4 @@ "d39e75f9" = "L1MessageNumberSynchronizationWrong" "7557a60a" = "L1RollingHashSynchronizationWrong" "36a4bb94" = "FinalRollingHashIsZero" +"42ab979d" = "ParentBlobNotSubmitted" diff --git a/contracts/abi/LineaRollupV6.0.abi b/contracts/abi/LineaRollupV6.0.abi index 5f8cd5442..b9ee6255a 100644 --- a/contracts/abi/LineaRollupV6.0.abi +++ b/contracts/abi/LineaRollupV6.0.abi @@ -52,27 +52,6 @@ "name": "DataAlreadySubmitted", "type": "error" }, - { - "inputs": [ - { - "internalType": "uint256", - "name": "expected", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "actual", - "type": "uint256" - } - ], - "name": "DataStartingBlockDoesNotMatch", - "type": "error" - }, - { - "inputs": [], - "name": "EmptyBlobData", - "type": "error" - }, { "inputs": [ { @@ -174,38 +153,6 @@ "name": "FinalizationStateIncorrect", "type": "error" }, - { - "inputs": [ - { - "internalType": "uint256", - "name": "firstBlockNumber", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "finalBlockNumber", - "type": "uint256" - } - ], - "name": "FirstBlockGreaterThanFinalBlock", - "type": "error" - }, - { - "inputs": [ - { - "internalType": "uint256", - "name": "firstBlockNumber", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "lastFinalizedBlock", - "type": "uint256" - } - ], - "name": "FirstBlockLessThanOrEqualToLastFinalizedBlock", - "type": "error" - }, { "inputs": [], "name": "FirstByteIsNotZero", @@ -296,22 +243,6 @@ "name": "LastFinalizationTimeNotLapsed", "type": "error" }, - { - "inputs": [ - { - "internalType": "bytes32", - "name": "expected", - "type": "bytes32" - }, - { - "internalType": "bytes32", - "name": "actual", - "type": "bytes32" - } - ], - "name": "LastFinalizedShnarfWrong", - "type": "error" - }, { "inputs": [ { @@ -388,6 +319,17 @@ "name": "MissingRollingHashForMessageNumber", "type": "error" }, + { + "inputs": [ + { + "internalType": "bytes32", + "name": "shnarf", + "type": "bytes32" + } + ], + "name": "ParentBlobNotSubmitted", + "type": "error" + }, { "inputs": [], "name": "PeriodIsZero", @@ -461,27 +403,6 @@ "name": "ReentrantCall", "type": "error" }, - { - "inputs": [ - { - "internalType": "uint256", - "name": "shnarfsLength", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "finalBlockNumbers", - "type": "uint256" - } - ], - "name": "ShnarfAndFinalBlockNumberLengthsMismatched", - "type": "error" - }, - { - "inputs": [], - "name": "SnarkHashIsZeroHash", - "type": "error" - }, { "inputs": [], "name": "StartingRootHashDoesNotMatch", @@ -555,18 +476,6 @@ { "anonymous": false, "inputs": [ - { - "indexed": true, - "internalType": "uint256", - "name": "startBlock", - "type": "uint256" - }, - { - "indexed": true, - "internalType": "uint256", - "name": "endBlock", - "type": "uint256" - }, { "indexed": false, "internalType": "bytes32", @@ -1315,6 +1224,25 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "bytes32", + "name": "shnarf", + "type": "bytes32" + } + ], + "name": "blobShnarfExists", + "outputs": [ + { + "internalType": "uint256", + "name": "exists", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -1656,7 +1584,7 @@ }, { "internalType": "uint256", - "name": "finalBlockInData", + "name": "endBlockNumber", "type": "uint256" }, { @@ -2296,25 +2224,6 @@ "stateMutability": "nonpayable", "type": "function" }, - { - "inputs": [ - { - "internalType": "bytes32", - "name": "shnarf", - "type": "bytes32" - } - ], - "name": "shnarfFinalBlockNumbers", - "outputs": [ - { - "internalType": "uint256", - "name": "finalBlockNumber", - "type": "uint256" - } - ], - "stateMutability": "view", - "type": "function" - }, { "inputs": [ { @@ -2338,33 +2247,6 @@ "inputs": [ { "components": [ - { - "components": [ - { - "internalType": "bytes32", - "name": "finalStateRootHash", - "type": "bytes32" - }, - { - "internalType": "uint256", - "name": "firstBlockInData", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "finalBlockInData", - "type": "uint256" - }, - { - "internalType": "bytes32", - "name": "snarkHash", - "type": "bytes32" - } - ], - "internalType": "struct ILineaRollup.SupportingSubmissionDataV2", - "name": "submissionData", - "type": "tuple" - }, { "internalType": "uint256", "name": "dataEvaluationClaim", @@ -2379,10 +2261,20 @@ "internalType": "bytes", "name": "kzgProof", "type": "bytes" + }, + { + "internalType": "bytes32", + "name": "finalStateRootHash", + "type": "bytes32" + }, + { + "internalType": "bytes32", + "name": "snarkHash", + "type": "bytes32" } ], - "internalType": "struct ILineaRollup.BlobSubmissionData[]", - "name": "_blobSubmissionData", + "internalType": "struct ILineaRollup.BlobSubmission[]", + "name": "_blobSubmissions", "type": "tuple[]" }, { @@ -2410,16 +2302,6 @@ "name": "finalStateRootHash", "type": "bytes32" }, - { - "internalType": "uint256", - "name": "firstBlockInData", - "type": "uint256" - }, - { - "internalType": "uint256", - "name": "finalBlockInData", - "type": "uint256" - }, { "internalType": "bytes32", "name": "snarkHash", @@ -2431,8 +2313,8 @@ "type": "bytes" } ], - "internalType": "struct ILineaRollup.SubmissionDataV2", - "name": "_submissionData", + "internalType": "struct ILineaRollup.CompressedCalldataSubmission", + "name": "_submission", "type": "tuple" }, { diff --git a/contracts/contracts/LineaRollup.sol b/contracts/contracts/LineaRollup.sol index 4fdaae747..5d55b52bb 100644 --- a/contracts/contracts/LineaRollup.sol +++ b/contracts/contracts/LineaRollup.sol @@ -40,6 +40,7 @@ contract LineaRollup is ) ); + uint256 internal constant SHNARF_EXISTS_DEFAULT_VALUE = 1; bytes32 internal constant EMPTY_HASH = 0x0; uint256 internal constant BLS_CURVE_MODULUS = 52435875175126190479447740508185965837690552500527637822603658699938581184513; @@ -49,15 +50,15 @@ contract LineaRollup is uint256 internal constant SIX_MONTHS_IN_SECONDS = (365 / 2) * 24 * 60 * 60; - /// @dev DEPRECATED in favor of the single shnarfFinalBlockNumbers mapping. + /// @dev DEPRECATED in favor of the single blobShnarfExists mapping. mapping(bytes32 dataHash => bytes32 finalStateRootHash) public dataFinalStateRootHashes; - /// @dev DEPRECATED in favor of the single shnarfFinalBlockNumbers mapping. + /// @dev DEPRECATED in favor of the single blobShnarfExists mapping. mapping(bytes32 dataHash => bytes32 parentHash) public dataParents; - /// @dev DEPRECATED in favor of the single shnarfFinalBlockNumbers mapping. + /// @dev DEPRECATED in favor of the single blobShnarfExists mapping. mapping(bytes32 dataHash => bytes32 shnarfHash) public dataShnarfHashes; - /// @dev DEPRECATED in favor of the single shnarfFinalBlockNumbers mapping. + /// @dev DEPRECATED in favor of the single blobShnarfExists mapping. mapping(bytes32 dataHash => uint256 startingBlock) public dataStartingBlock; - /// @dev DEPRECATED in favor of the single shnarfFinalBlockNumbers mapping. + /// @dev DEPRECATED in favor of the single blobShnarfExists mapping. mapping(bytes32 dataHash => uint256 endingBlock) public dataEndingBlock; /// @dev DEPRECATED in favor of currentFinalizedState hash. @@ -69,8 +70,9 @@ contract LineaRollup is /** * @dev NB: THIS IS THE ONLY MAPPING BEING USED FOR DATA SUBMISSION TRACKING. + * @dev NB: NB: This was shnarfFinalBlockNumbers and is replaced to indicate only that a shnarf exists with a value of 1. */ - mapping(bytes32 shnarf => uint256 finalBlockNumber) public shnarfFinalBlockNumbers; + mapping(bytes32 shnarf => uint256 exists) public blobShnarfExists; /// @dev Hash of the L2 computed L1 message number, rolling hash and finalized timestamp. bytes32 public currentFinalizedState; @@ -118,7 +120,7 @@ contract LineaRollup is currentL2BlockNumber = _initializationData.initialL2BlockNumber; stateRootHashes[_initializationData.initialL2BlockNumber] = _initializationData.initialStateRootHash; - shnarfFinalBlockNumbers[GENESIS_SHNARF] = _initializationData.initialL2BlockNumber; + blobShnarfExists[GENESIS_SHNARF] = SHNARF_EXISTS_DEFAULT_VALUE; currentFinalizedShnarf = GENESIS_SHNARF; currentFinalizedState = _computeLastFinalizedState(0, EMPTY_HASH, _initializationData.genesisTimestamp); @@ -203,16 +205,16 @@ contract LineaRollup is * @notice Submit one or more EIP-4844 blobs. * @dev OPERATOR_ROLE is required to execute. * @dev This should be a blob carrying transaction. - * @param _blobSubmissionData The data for blob submission including proofs and required polynomials. + * @param _blobSubmissions The data for blob submission including proofs and required polynomials. * @param _parentShnarf The parent shnarf used in continuity checks as it includes the parentStateRootHash in its computation. * @param _finalBlobShnarf The expected final shnarf post computation of all the blob shnarfs. */ function submitBlobs( - BlobSubmissionData[] calldata _blobSubmissionData, + BlobSubmission[] calldata _blobSubmissions, bytes32 _parentShnarf, bytes32 _finalBlobShnarf ) external whenTypeAndGeneralNotPaused(PauseType.BLOB_SUBMISSION) onlyRole(OPERATOR_ROLE) { - uint256 blobSubmissionLength = _blobSubmissionData.length; + uint256 blobSubmissionLength = _blobSubmissions.length; if (blobSubmissionLength == 0) { revert BlobSubmissionDataIsMissing(); @@ -224,17 +226,18 @@ contract LineaRollup is bytes32 currentDataEvaluationPoint; bytes32 currentDataHash; - uint256 lastFinalizedBlockNumber = currentL2BlockNumber; /// @dev Assigning in memory saves a lot of gas vs. calldata reading. - BlobSubmissionData memory blobSubmissionData; + BlobSubmission memory blobSubmission; - bytes32 computedShnarf = _parentShnarf; + if (blobShnarfExists[_parentShnarf] == 0) { + revert ParentBlobNotSubmitted(_parentShnarf); + } - uint256 blobFinalBlockNumber = shnarfFinalBlockNumbers[computedShnarf]; + bytes32 computedShnarf = _parentShnarf; for (uint256 i; i < blobSubmissionLength; i++) { - blobSubmissionData = _blobSubmissionData[i]; + blobSubmission = _blobSubmissions[i]; currentDataHash = blobhash(i); @@ -242,27 +245,25 @@ contract LineaRollup is revert EmptyBlobDataAtIndex(i); } - _validateSubmissionData(blobSubmissionData.submissionData, blobFinalBlockNumber, lastFinalizedBlockNumber); + bytes32 snarkHash = blobSubmission.snarkHash; - currentDataEvaluationPoint = Utils._efficientKeccak(blobSubmissionData.submissionData.snarkHash, currentDataHash); + currentDataEvaluationPoint = Utils._efficientKeccak(snarkHash, currentDataHash); _verifyPointEvaluation( currentDataHash, uint256(currentDataEvaluationPoint), - blobSubmissionData.dataEvaluationClaim, - blobSubmissionData.kzgCommitment, - blobSubmissionData.kzgProof + blobSubmission.dataEvaluationClaim, + blobSubmission.kzgCommitment, + blobSubmission.kzgProof ); computedShnarf = _computeShnarf( computedShnarf, - blobSubmissionData.submissionData.snarkHash, - blobSubmissionData.submissionData.finalStateRootHash, + snarkHash, + blobSubmission.finalStateRootHash, currentDataEvaluationPoint, - bytes32(blobSubmissionData.dataEvaluationClaim) + bytes32(blobSubmission.dataEvaluationClaim) ); - - blobFinalBlockNumber = blobSubmissionData.submissionData.finalBlockInData; } if (_finalBlobShnarf != computedShnarf) { @@ -274,110 +275,55 @@ contract LineaRollup is * Note: As only the last shnarf is stored, we don't need to validate shnarfs, * computed for any previous blobs in the submission (if multiple are submitted). */ - if (shnarfFinalBlockNumbers[computedShnarf] != 0) { + if (blobShnarfExists[computedShnarf] != 0) { revert DataAlreadySubmitted(computedShnarf); } /// @dev use the last shnarf as the submission to store as technically it becomes the next parent shnarf. - shnarfFinalBlockNumbers[computedShnarf] = blobFinalBlockNumber; + blobShnarfExists[computedShnarf] = SHNARF_EXISTS_DEFAULT_VALUE; - emit DataSubmittedV3( - _blobSubmissionData[0].submissionData.firstBlockInData, - blobFinalBlockNumber, - _parentShnarf, - computedShnarf, - blobSubmissionData.submissionData.finalStateRootHash - ); + emit DataSubmittedV3(_parentShnarf, computedShnarf, blobSubmission.finalStateRootHash); } /** * @notice Submit blobs using compressed data via calldata. * @dev OPERATOR_ROLE is required to execute. - * @param _submissionData The supporting data for compressed data submission including compressed data. + * @param _submission The supporting data for compressed data submission including compressed data. * @param _parentShnarf The parent shnarf used in continuity checks as it includes the parentStateRootHash in its computation. * @param _expectedShnarf The expected shnarf post computation of all the submission. */ function submitDataAsCalldata( - SubmissionDataV2 calldata _submissionData, + CompressedCalldataSubmission calldata _submission, bytes32 _parentShnarf, bytes32 _expectedShnarf ) external whenTypeAndGeneralNotPaused(PauseType.CALLDATA_SUBMISSION) onlyRole(OPERATOR_ROLE) { - if (_submissionData.compressedData.length == 0) { + if (_submission.compressedData.length == 0) { revert EmptySubmissionData(); } - SupportingSubmissionDataV2 memory submissionData = SupportingSubmissionDataV2({ - finalStateRootHash: _submissionData.finalStateRootHash, - firstBlockInData: _submissionData.firstBlockInData, - finalBlockInData: _submissionData.finalBlockInData, - snarkHash: _submissionData.snarkHash - }); + bytes32 currentDataHash = keccak256(_submission.compressedData); - bytes32 currentDataHash = keccak256(_submissionData.compressedData); + bytes32 dataEvaluationPoint = Utils._efficientKeccak(_submission.snarkHash, currentDataHash); - _validateSubmissionData(submissionData, shnarfFinalBlockNumbers[_parentShnarf], currentL2BlockNumber); - - bytes32 dataEvaluationPoint = Utils._efficientKeccak(_submissionData.snarkHash, currentDataHash); bytes32 computedShnarf = _computeShnarf( _parentShnarf, - _submissionData.snarkHash, - _submissionData.finalStateRootHash, + _submission.snarkHash, + _submission.finalStateRootHash, dataEvaluationPoint, - _calculateY(_submissionData.compressedData, dataEvaluationPoint) + _calculateY(_submission.compressedData, dataEvaluationPoint) ); if (_expectedShnarf != computedShnarf) { revert FinalShnarfWrong(_expectedShnarf, computedShnarf); } - if (shnarfFinalBlockNumbers[computedShnarf] != 0) { + if (blobShnarfExists[computedShnarf] != 0) { revert DataAlreadySubmitted(computedShnarf); } - shnarfFinalBlockNumbers[computedShnarf] = _submissionData.finalBlockInData; + blobShnarfExists[computedShnarf] = SHNARF_EXISTS_DEFAULT_VALUE; - emit DataSubmittedV3( - _submissionData.firstBlockInData, - _submissionData.finalBlockInData, - _parentShnarf, - computedShnarf, - _submissionData.finalStateRootHash - ); - } - - /** - * @notice Internal function to validate submission data. - * @param _submissionData The supporting data for compressed data submission excluding compressed data. - * @param _parentFinalBlockNumber The final block number for the parent blob. - * @param _lastFinalizedBlockNumber The last finalized block number. - */ - function _validateSubmissionData( - SupportingSubmissionDataV2 memory _submissionData, - uint256 _parentFinalBlockNumber, - uint256 _lastFinalizedBlockNumber - ) internal pure { - if (_submissionData.finalStateRootHash == EMPTY_HASH) { - revert FinalBlockStateEqualsZeroHash(); - } - - if (_submissionData.snarkHash == EMPTY_HASH) { - revert SnarkHashIsZeroHash(); - } - - // for it to be equal the number would have to wrap round twice in overflow.. - unchecked { - if (_parentFinalBlockNumber + 1 != _submissionData.firstBlockInData) { - revert DataStartingBlockDoesNotMatch(_parentFinalBlockNumber + 1, _submissionData.firstBlockInData); - } - } - - if (_submissionData.firstBlockInData <= _lastFinalizedBlockNumber) { - revert FirstBlockLessThanOrEqualToLastFinalizedBlock(_submissionData.firstBlockInData, _lastFinalizedBlockNumber); - } - - if (_submissionData.firstBlockInData > _submissionData.finalBlockInData) { - revert FirstBlockGreaterThanFinalBlock(_submissionData.firstBlockInData, _submissionData.finalBlockInData); - } + emit DataSubmittedV3(_parentShnarf, computedShnarf, _submission.finalStateRootHash); } /** @@ -502,7 +448,7 @@ contract LineaRollup is lastFinalizedShnarf, finalShnarf, lastFinalizedBlockNumber, - shnarfFinalBlockNumbers[finalShnarf] + _finalizationData.endBlockNumber ); _verifyProof(publicInput, _proofType, _aggregatedProof); @@ -518,11 +464,8 @@ contract LineaRollup is FinalizationDataV3 calldata _finalizationData, uint256 _lastFinalizedBlock ) internal returns (bytes32 finalShnarf) { - if (_finalizationData.finalBlockInData <= _lastFinalizedBlock) { - revert FinalBlockNumberLessThanOrEqualToLastFinalizedBlock( - _finalizationData.finalBlockInData, - _lastFinalizedBlock - ); + if (_finalizationData.endBlockNumber <= _lastFinalizedBlock) { + revert FinalBlockNumberLessThanOrEqualToLastFinalizedBlock(_finalizationData.endBlockNumber, _lastFinalizedBlock); } _validateL2ComputedRollingHash(_finalizationData.l1RollingHashMessageNumber, _finalizationData.l1RollingHash); @@ -563,9 +506,9 @@ contract LineaRollup is _addL2MerkleRoots(_finalizationData.l2MerkleRoots, _finalizationData.l2MerkleTreesDepth); _anchorL2MessagingBlocks(_finalizationData.l2MessagingBlocksOffsets, _lastFinalizedBlock); - stateRootHashes[_finalizationData.finalBlockInData] = _finalizationData.shnarfData.finalStateRootHash; + stateRootHashes[_finalizationData.endBlockNumber] = _finalizationData.shnarfData.finalStateRootHash; - currentL2BlockNumber = _finalizationData.finalBlockInData; + currentL2BlockNumber = _finalizationData.endBlockNumber; currentFinalizedShnarf = finalShnarf; @@ -576,9 +519,8 @@ contract LineaRollup is ); emit DataFinalizedV3( - /// @dev incremented to cover the starting block of data being finalized ++_lastFinalizedBlock, - _finalizationData.finalBlockInData, + _finalizationData.endBlockNumber, finalShnarf, _finalizationData.parentStateRootHash, _finalizationData.shnarfData.finalStateRootHash @@ -657,7 +599,7 @@ contract LineaRollup is * _finalizationData.lastFinalizedTimestamp, * _finalizationData.finalTimestamp, * _lastFinalizedBlockNumber, - * _finalizationData.finalBlockInData, + * _finalizationData.endBlockNumber, * _finalizationData.lastFinalizedL1RollingHash, * _finalizationData.l1RollingHash, * _finalizationData.lastFinalizedL1RollingHashMessageNumber, @@ -670,7 +612,7 @@ contract LineaRollup is * ) * Data is found at the following offsets: * 0x00 parentStateRootHash - * 0x20 finalBlockInData + * 0x20 endBlockNumber * 0x40 shnarfData.parentShnarf * 0x60 shnarfData.snarkHash * 0x80 shnarfData.finalStateRootHash @@ -692,14 +634,14 @@ contract LineaRollup is * @param _finalizationData The full finalization data. * @param _finalShnarf The final shnarf in the finalization. * @param _lastFinalizedBlockNumber The last finalized block number. - * @param _finalBlockNumber Final block number being finalized. + * @param _endBlockNumber End block number being finalized. */ function _computePublicInput( FinalizationDataV3 calldata _finalizationData, bytes32 _lastFinalizedShnarf, bytes32 _finalShnarf, uint256 _lastFinalizedBlockNumber, - uint256 _finalBlockNumber + uint256 _endBlockNumber ) private pure returns (uint256 publicInput) { assembly { let mPtr := mload(0x40) @@ -714,8 +656,8 @@ contract LineaRollup is mstore(add(mPtr, 0x80), _lastFinalizedBlockNumber) - // shnarfFinalBlockNumbers[finalShnarf] - mstore(add(mPtr, 0xA0), _finalBlockNumber) + // _finalizationData.endBlockNumber + mstore(add(mPtr, 0xA0), _endBlockNumber) /** * _finalizationData.lastFinalizedL1RollingHash diff --git a/contracts/contracts/interfaces/l1/ILineaRollup.sol b/contracts/contracts/interfaces/l1/ILineaRollup.sol index 9f93133bd..c93f16a9c 100644 --- a/contracts/contracts/interfaces/l1/ILineaRollup.sol +++ b/contracts/contracts/interfaces/l1/ILineaRollup.sol @@ -40,34 +40,16 @@ interface ILineaRollup { /** * @notice Supporting data for compressed calldata submission including compressed data. - * @dev finalStateRootHash is used to set state root hash at the end of the data. - * @dev firstBlockInData is the first block that is included in the data submitted. - * @dev finalBlockInData is the last block that is included in the data submitted. + * @dev finalStateRootHash is used to set state root at the end of the data. * @dev snarkHash is the computed hash for compressed data (using a SNARK-friendly hash function) that aggregates per data submission to be used in public input. * @dev compressedData is the compressed transaction data. It contains ordered data for each L2 block - l2Timestamps, the encoded transaction data. */ - struct SubmissionDataV2 { + struct CompressedCalldataSubmission { bytes32 finalStateRootHash; - uint256 firstBlockInData; - uint256 finalBlockInData; bytes32 snarkHash; bytes compressedData; } - /** - * @notice Supporting data for compressed blob data submission. - * @dev finalStateRootHash is used to set state root at the end of the data. - * @dev firstBlockInData is the first block that is included in the data submitted. - * @dev finalBlockInData is the last block that is included in the data submitted. - * @dev snarkHash is the computed hash for compressed data (using a SNARK-friendly hash function) that aggregates per data submission to be used in public input. - */ - struct SupportingSubmissionDataV2 { - bytes32 finalStateRootHash; - uint256 firstBlockInData; - uint256 finalBlockInData; - bytes32 snarkHash; - } - /** * @notice Shnarf data for validating a shnarf. * @dev parentShnarf is the parent computed shnarf. @@ -91,18 +73,19 @@ interface ILineaRollup { * @dev kzgCommitment The blob KZG commitment. * @dev kzgProof The blob KZG point proof. */ - struct BlobSubmissionData { - SupportingSubmissionDataV2 submissionData; + struct BlobSubmission { uint256 dataEvaluationClaim; bytes kzgCommitment; bytes kzgProof; + bytes32 finalStateRootHash; + bytes32 snarkHash; } /** * @notice Supporting data for finalization with proof. * @dev NB: the dynamic sized fields are placed last on purpose for efficient keccaking on public input. * @dev parentStateRootHash is the expected last state root hash finalized. - * @dev finalBlockInData is the final block finalizing until. + * @dev endBlockNumber is the end block finalizing until. * @dev shnarfData contains data about the last data submission's shnarf used in finalization. * @dev lastFinalizedTimestamp is the expected last finalized block's timestamp. * @dev finalTimestamp is the timestamp of the last block being finalized. @@ -113,12 +96,12 @@ interface ILineaRollup { * @dev l1RollingHashMessageNumber is the calculated message number on L2 that is expected to match the existing L1 rolling hash. * This value will be used along with the stored last finalized L2 calculated message number in the public input. * @dev l2MerkleTreesDepth is the depth of all l2MerkleRoots. - * @dev l2MerkleRoots is an array of L2 message Merkle roots of depth l2MerkleTreesDepth between last finalized block and finalSubmissionData.finalBlockInData. + * @dev l2MerkleRoots is an array of L2 message Merkle roots of depth l2MerkleTreesDepth between last finalized block and finalSubmissionData.finalBlockNumber. * @dev l2MessagingBlocksOffsets indicates by offset from currentL2BlockNumber which L2 blocks contain MessageSent events. */ struct FinalizationDataV3 { bytes32 parentStateRootHash; - uint256 finalBlockInData; + uint256 endBlockNumber; ShnarfData shnarfData; uint256 lastFinalizedTimestamp; uint256 finalTimestamp; @@ -172,19 +155,11 @@ interface ILineaRollup { /** * @notice Emitted when compressed data is being submitted and verified succesfully on L1. * @dev The block range is indexed and parent shnarf included for state reconstruction simplicity. - * @param startBlock The indexed L2 block number indicating which block the data starts from. - * @param endBlock The indexed L2 block number indicating which block the data ends on. * @param parentShnarf The parent shnarf for the data being submitted. * @param shnarf The indexed shnarf for the data being submitted. * @param finalStateRootHash The L2 state root hash that the current blob submission ends on. NB: The last blob in the collection. */ - event DataSubmittedV3( - uint256 indexed startBlock, - uint256 indexed endBlock, - bytes32 parentShnarf, - bytes32 indexed shnarf, - bytes32 finalStateRootHash - ); + event DataSubmittedV3(bytes32 parentShnarf, bytes32 indexed shnarf, bytes32 finalStateRootHash); /** * @notice Emitted when L2 blocks have been finalized on L1. @@ -222,11 +197,6 @@ interface ILineaRollup { */ error PointEvaluationFailed(); - /** - * @dev Thrown when the blobhash equals the zero hash. - */ - error EmptyBlobData(); - /** * @dev Thrown when the blobhash at an index equals to the zero hash. */ @@ -242,21 +212,11 @@ interface ILineaRollup { */ error BlobSubmissionDataEmpty(uint256 emptyBlobIndex); - /** - * @dev Thrown when the starting block in the data item is out of sequence with the last block number. - */ - error DataStartingBlockDoesNotMatch(uint256 expected, uint256 actual); - /** * @dev Thrown when the current data was already submitted. */ error DataAlreadySubmitted(bytes32 currentDataHash); - /** - * @dev Thrown when the last finalized shnarf does not match the parent finalizing from. - */ - error LastFinalizedShnarfWrong(bytes32 expected, bytes32 actual); - /** * @dev Thrown when submissionData is empty. */ @@ -272,16 +232,6 @@ interface ILineaRollup { */ error FinalizationStateIncorrect(bytes32 expected, bytes32 value); - /** - * @dev Thrown when the first block is greater than final block in submission data. - */ - error FirstBlockGreaterThanFinalBlock(uint256 firstBlockNumber, uint256 finalBlockNumber); - - /** - * @dev Thrown when the first block in data is less than or equal to the last finalized block during data submission. - */ - error FirstBlockLessThanOrEqualToLastFinalizedBlock(uint256 firstBlockNumber, uint256 lastFinalizedBlock); - /** * @dev Thrown when the final block number in finalization data is less than or equal to the last finalized block during finalization. */ @@ -319,14 +269,14 @@ interface ILineaRollup { error BytesLengthNotMultipleOf32(); /** - * @dev Thrown when the snarkhash is the zero hash. + * @dev Thrown when the computed shnarf does not match what is expected. */ - error SnarkHashIsZeroHash(); + error FinalShnarfWrong(bytes32 expected, bytes32 value); /** - * @dev Thrown when the computed shnarf does not match what is expected. + * @dev Thrown when a shnarf does not exist for a parent blob. */ - error FinalShnarfWrong(bytes32 expected, bytes32 value); + error ParentBlobNotSubmitted(bytes32 shnarf); /** * @notice Adds or updates the verifier contract address for a proof type. @@ -356,12 +306,12 @@ interface ILineaRollup { * @notice Submit one or more EIP-4844 blobs. * @dev OPERATOR_ROLE is required to execute. * @dev This should be a blob carrying transaction. - * @param _blobSubmissionData The data for blob submission including proofs and required polynomials. + * @param _blobSubmissions The data for blob submission including proofs and required polynomials. * @param _parentShnarf The parent shnarf used in continuity checks as it includes the parentStateRootHash in its computation. * @param _finalBlobShnarf The expected final shnarf post computation of all the blob shnarfs. */ function submitBlobs( - BlobSubmissionData[] calldata _blobSubmissionData, + BlobSubmission[] calldata _blobSubmissions, bytes32 _parentShnarf, bytes32 _finalBlobShnarf ) external; @@ -369,12 +319,12 @@ interface ILineaRollup { /** * @notice Submit blobs using compressed data via calldata. * @dev OPERATOR_ROLE is required to execute. - * @param _submissionData The supporting data for compressed data submission including compressed data. + * @param _submission The supporting data for compressed data submission including compressed data. * @param _parentShnarf The parent shnarf used in continuity checks as it includes the parentStateRootHash in its computation. * @param _expectedShnarf The expected shnarf post computation of all the submission. */ function submitDataAsCalldata( - SubmissionDataV2 calldata _submissionData, + CompressedCalldataSubmission calldata _submission, bytes32 _parentShnarf, bytes32 _expectedShnarf ) external; diff --git a/contracts/contracts/test-contracts/TestLineaRollup.sol b/contracts/contracts/test-contracts/TestLineaRollup.sol index 013ca09ca..17083ba45 100644 --- a/contracts/contracts/test-contracts/TestLineaRollup.sol +++ b/contracts/contracts/test-contracts/TestLineaRollup.sol @@ -24,8 +24,8 @@ contract TestLineaRollup is LineaRollup { return _calculateY(_data, _x); } - function setupParentShnarf(bytes32 _shnarf, uint256 _finalBlockNumber) external { - shnarfFinalBlockNumbers[_shnarf] = _finalBlockNumber; + function setupParentShnarf(bytes32 _shnarf) external { + blobShnarfExists[_shnarf] = 1; } function setupParentDataShnarf(bytes32 _parentDataHash, bytes32 _shnarf) external { @@ -49,6 +49,6 @@ contract TestLineaRollup is LineaRollup { } function setShnarfFinalBlockNumber(bytes32 _shnarf, uint256 _finalBlockNumber) external { - shnarfFinalBlockNumbers[_shnarf] = _finalBlockNumber; + blobShnarfExists[_shnarf] = _finalBlockNumber; } } diff --git a/contracts/scripts/testEIP4844/SixInOne/sendBlobTransaction.ts b/contracts/scripts/testEIP4844/SixInOne/sendBlobTransaction.ts index 2ed8bfcee..4ccdfbe02 100644 --- a/contracts/scripts/testEIP4844/SixInOne/sendBlobTransaction.ts +++ b/contracts/scripts/testEIP4844/SixInOne/sendBlobTransaction.ts @@ -209,23 +209,21 @@ async function main() { // eslint-disable-next-line @typescript-eslint/no-explicit-any function mapToTuple(blobSubmissionDataItems: BlobSubmissionData[]): any { return blobSubmissionDataItems.map((blobSubmissionData) => [ - [ - blobSubmissionData.submissionData.finalStateRootHash, - blobSubmissionData.submissionData.firstBlockInData, - blobSubmissionData.submissionData.finalBlockInData, - blobSubmissionData.submissionData.snarkHash, - ], blobSubmissionData.dataEvaluationClaim, blobSubmissionData.kzgCommitment, blobSubmissionData.kzgProof, + blobSubmissionData.submissionData.finalStateRootHash, + blobSubmissionData.submissionData.snarkHash, ]); } function encodeCall(submissionData: BlobSubmissionData[]): DataHexString { + //submitBlobs((uint256,bytes,bytes,bytes32,bytes32)[],bytes32,bytes32)": "99467a35" + const encodedCall = ethers.concat([ - "0x42fbe842", + "0x99467a35", ethers.AbiCoder.defaultAbiCoder().encode( - ["tuple(tuple(bytes32,uint256,uint256,bytes32),uint256,bytes,bytes)[]", "bytes32", "bytes32"], + ["tuple(uint256,bytes,bytes,bytes32,bytes32)[]", "bytes32", "bytes32"], [ mapToTuple(submissionData), submissionData[0].parentSubmissionData.shnarf, @@ -354,9 +352,10 @@ async function sendProof( ]; console.log(proofData); - + //finalizeBlocks(bytes,uint256,(bytes32,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32),uint256,uint256,bytes32,bytes32,uint256,uint256,uint256,bytes32[],bytes))": "5603c65f" + //finalizeBlocks(bytes,uint256,(bytes32,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32),uint256,uint256,bytes32,bytes32,uint256,uint256,uint256,bytes32[],bytes))": "5603c65f" const encodedCall = ethers.concat([ - "0x227be0dc", + "0x5603c65f", ethers.AbiCoder.defaultAbiCoder().encode( [ "bytes", diff --git a/contracts/test/LineaRollup.ts b/contracts/test/LineaRollup.ts index be6600db8..6edd28b3d 100644 --- a/contracts/test/LineaRollup.ts +++ b/contracts/test/LineaRollup.ts @@ -400,8 +400,8 @@ describe("Linea Rollup contract", () => { .submitDataAsCalldata(submissionData, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }), ).to.not.be.reverted; - const finalBlockNumber = await lineaRollup.shnarfFinalBlockNumbers(expectedShnarf); - expect(finalBlockNumber).to.equal(submissionData.finalBlockInData); + const blobShnarfExists = await lineaRollup.blobShnarfExists(expectedShnarf); + expect(blobShnarfExists).to.equal(1n); }); it("Should successfully submit 2 compressed data chunks in two transactions", async () => { @@ -419,9 +419,8 @@ describe("Linea Rollup contract", () => { }), ).to.not.be.reverted; - const finalBlockNumber = await lineaRollup.shnarfFinalBlockNumbers(secondExpectedShnarf); - - expect(finalBlockNumber).to.equal(secondSubmissionData.finalBlockInData); + const blobShnarfExists = await lineaRollup.blobShnarfExists(expectedShnarf); + expect(blobShnarfExists).to.equal(1n); }); it("Should emit an event while submitting 1 compressed data chunk", async () => { @@ -430,30 +429,11 @@ describe("Linea Rollup contract", () => { const submitDataCall = lineaRollup .connect(operator) .submitDataAsCalldata(submissionData, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }); - const eventArgs = [ - submissionData.firstBlockInData, - submissionData.finalBlockInData, - prevShnarf, - expectedShnarf, - submissionData.finalStateRootHash, - ]; + const eventArgs = [prevShnarf, expectedShnarf, submissionData.finalStateRootHash]; await expectEvent(lineaRollup, submitDataCall, "DataSubmittedV3", eventArgs); }); - it("Should fail if the stored shnarf block number + 1 does not match the starting submission number", async () => { - const [submissionData] = generateCallDataSubmission(0, 1); - - await lineaRollup.setShnarfFinalBlockNumber(prevShnarf, 99n); - - const submitDataCall = lineaRollup - .connect(operator) - .submitDataAsCalldata(submissionData, prevShnarf, secondExpectedShnarf, { gasLimit: 30_000_000 }); - const eventArgs = [100n, 1n]; - - await expectRevertWithCustomError(lineaRollup, submitDataCall, "DataStartingBlockDoesNotMatch", eventArgs); - }); - it("Should fail if the final state root hash is empty", async () => { const [submissionData] = generateCallDataSubmission(0, 1); @@ -463,27 +443,11 @@ describe("Linea Rollup contract", () => { .connect(operator) .submitDataAsCalldata(submissionData, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }); - await expectRevertWithCustomError(lineaRollup, submitDataCall, "FinalBlockStateEqualsZeroHash"); - }); - - it("Should fail if the block numbers are out of sequence", async () => { - const [firstSubmissionData, secondSubmissionData] = generateCallDataSubmission(0, 2); - - await expect( - lineaRollup - .connect(operator) - .submitDataAsCalldata(firstSubmissionData, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }), - ).to.not.be.reverted; - - const expectedFirstBlock = secondSubmissionData.firstBlockInData; - secondSubmissionData.firstBlockInData = secondSubmissionData.firstBlockInData + 1n; - - const submitDataCall = lineaRollup - .connect(operator) - .submitDataAsCalldata(secondSubmissionData, expectedShnarf, secondExpectedShnarf, { gasLimit: 30_000_000 }); - const eventArgs = [expectedFirstBlock, secondSubmissionData.firstBlockInData]; - - await expectRevertWithCustomError(lineaRollup, submitDataCall, "DataStartingBlockDoesNotMatch", eventArgs); + // TODO: Make the failure shnarf dynamic and computed + await expectRevertWithCustomError(lineaRollup, submitDataCall, "FinalShnarfWrong", [ + expectedShnarf, + "0xf53c28b2287f506b4df1b9de48cf3601392d54a73afe400a6f8f4ded2e0929ad", + ]); }); it("Should fail to submit where expected shnarf is wrong", async () => { @@ -534,36 +498,6 @@ describe("Linea Rollup contract", () => { await expectRevertWithCustomError(lineaRollup, submitDataCall, "IsPaused", [CALLDATA_SUBMISSION_PAUSE_TYPE]); }); - it("Should revert with FirstBlockLessThanOrEqualToLastFinalizedBlock when submitting data with firstBlockInData less than currentL2BlockNumber", async () => { - await lineaRollup.setLastFinalizedBlock(1_000_000); - - const submitDataCall = lineaRollup - .connect(operator) - .submitDataAsCalldata(DATA_ONE, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }); - - await expectRevertWithCustomError(lineaRollup, submitDataCall, "FirstBlockLessThanOrEqualToLastFinalizedBlock", [ - DATA_ONE.firstBlockInData, - 1000000, - ]); - }); - - it("Should revert with FirstBlockGreaterThanFinalBlock when submitting data with firstBlockInData greater than finalBlockInData", async () => { - const submissionData: CalldataSubmissionData = { - ...DATA_ONE, - firstBlockInData: DATA_ONE.firstBlockInData, - finalBlockInData: DATA_ONE.firstBlockInData - 1n, - }; - - const submitDataCall = lineaRollup - .connect(operator) - .submitDataAsCalldata(submissionData, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }); - - await expectRevertWithCustomError(lineaRollup, submitDataCall, "FirstBlockGreaterThanFinalBlock", [ - submissionData.firstBlockInData, - submissionData.finalBlockInData, - ]); - }); - it("Should revert with DataAlreadySubmitted when submitting same compressed data twice in 2 separate transactions", async () => { await lineaRollup .connect(operator) @@ -582,7 +516,7 @@ describe("Linea Rollup contract", () => { .submitDataAsCalldata(DATA_ONE, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }); const [dataOneCopy] = generateCallDataSubmission(0, 1); - dataOneCopy.finalBlockInData = 234253242n; + dataOneCopy.endBlockNumber = 234253242n; const submitDataCall = lineaRollup .connect(operator) @@ -601,14 +535,18 @@ describe("Linea Rollup contract", () => { .connect(operator) .submitDataAsCalldata(submissionData, prevShnarf, expectedShnarf, { gasLimit: 30_000_000 }); - await expectRevertWithCustomError(lineaRollup, submitDataCall, "SnarkHashIsZeroHash"); + // TODO: Make the failure shnarf dynamic and computed + await expectRevertWithCustomError(lineaRollup, submitDataCall, "FinalShnarfWrong", [ + expectedShnarf, + "0xa6b52564082728b51bb81a4fa92cfb4ec3af8de3f18b5d68ec27b89eead93293", + ]); }); }); describe("EIP-4844 Blob submission tests", () => { beforeEach(async () => { await lineaRollup.setLastFinalizedBlock(0); - await lineaRollup.setupParentShnarf(prevShnarf, 0); + await lineaRollup.setupParentShnarf(prevShnarf); await lineaRollup.setupParentDataShnarf(parentDataHash, prevShnarf); await lineaRollup.setupParentFinalizedStateRoot(parentDataHash, parentStateRootHash); }); @@ -649,17 +587,56 @@ describe("Linea Rollup contract", () => { expect(receipt).is.not.null; const expectedEventArgs = [ - blobDataSubmission[0].submissionData.firstBlockInData, - blobDataSubmission[0].submissionData.finalBlockInData, parentShnarf, finalShnarf, - blobDataSubmission[blobDataSubmission.length - 1].submissionData.finalStateRootHash, + blobDataSubmission[blobDataSubmission.length - 1].finalStateRootHash, ]; expectEventDirectFromReceiptData(lineaRollup as BaseContract, receipt!, "DataSubmittedV3", expectedEventArgs); - const finalBlockNumber = await lineaRollup.shnarfFinalBlockNumbers(finalShnarf); - expect(finalBlockNumber).to.equal(blobDataSubmission[0].submissionData.finalBlockInData); + const blobShnarfExists = await lineaRollup.blobShnarfExists(finalShnarf); + expect(blobShnarfExists).to.equal(1n); + }); + + it("Fails the blob submission when the parent shnarf is missing", async () => { + const operatorHDSigner = getWalletForIndex(2); + + const lineaRollupAddress = await lineaRollup.getAddress(); + const { blobDataSubmission, compressedBlobs, finalShnarf } = generateBlobDataSubmission(0, 1); + const nonExistingParentShnarf = generateRandomBytes(32); + + const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ + blobDataSubmission, + nonExistingParentShnarf, + finalShnarf, + ]); + + const { maxFeePerGas, maxPriorityFeePerGas } = await ethers.provider.getFeeData(); + const nonce = await operatorHDSigner.getNonce(); + + const transaction = Transaction.from({ + data: encodedCall, + maxPriorityFeePerGas: maxPriorityFeePerGas!, + maxFeePerGas: maxFeePerGas!, + to: lineaRollupAddress, + chainId: (await ethers.provider.getNetwork()).chainId, + type: 3, + nonce, + value: 0, + gasLimit: 5_000_000, + kzg, + maxFeePerBlobGas: 1n, + blobs: compressedBlobs, + }); + + const signedTx = await operatorHDSigner.signTransaction(transaction); + + await expectRevertWithCustomError( + lineaRollup, + ethers.provider.broadcastTransaction(signedTx), + "ParentBlobNotSubmitted", + [nonExistingParentShnarf], + ); }); it("Fails when the blob submission data is missing", async () => { @@ -776,7 +753,7 @@ describe("Linea Rollup contract", () => { const lineaRollupAddress = await lineaRollup.getAddress(); const { blobDataSubmission, compressedBlobs, parentShnarf, finalShnarf } = generateBlobDataSubmission(0, 1); - blobDataSubmission[0].submissionData.finalStateRootHash = HASH_ZERO; + blobDataSubmission[0].finalStateRootHash = HASH_ZERO; const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ blobDataSubmission, @@ -804,10 +781,12 @@ describe("Linea Rollup contract", () => { const signedTx = await operatorHDSigner.signTransaction(transaction); + // TODO: Make the failure shnarf dynamic and computed await expectRevertWithCustomError( lineaRollup, ethers.provider.broadcastTransaction(signedTx), - "FinalBlockStateEqualsZeroHash", + "FinalShnarfWrong", + [finalShnarf, "0x22f8fb954df8328627fe9c48b60192f4d970a92891417aaadea39300ca244d36"], ); }); @@ -819,96 +798,7 @@ describe("Linea Rollup contract", () => { // Set the snarkHash to HASH_ZERO for a specific index const emptyDataIndex = 0; - blobDataSubmission[emptyDataIndex].submissionData.snarkHash = HASH_ZERO; - - const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ - blobDataSubmission, - parentShnarf, - finalShnarf, - ]); - - const { maxFeePerGas, maxPriorityFeePerGas } = await ethers.provider.getFeeData(); - const nonce = await operatorHDSigner.getNonce(); - - const transaction = Transaction.from({ - data: encodedCall, - maxPriorityFeePerGas: maxPriorityFeePerGas!, - maxFeePerGas: maxFeePerGas!, - to: lineaRollupAddress, - chainId: (await ethers.provider.getNetwork()).chainId, - type: 3, - nonce, - value: 0, - gasLimit: 5_000_000, - kzg, - maxFeePerBlobGas: 1n, - blobs: compressedBlobs, - }); - - const signedTx = await operatorHDSigner.signTransaction(transaction); - - await expectRevertWithCustomError( - lineaRollup, - ethers.provider.broadcastTransaction(signedTx), - "SnarkHashIsZeroHash", - ); - }); - - it("Should fail if the block numbers are out of sequence", async () => { - const operatorHDSigner = getWalletForIndex(2); - - const lineaRollupAddress = await lineaRollup.getAddress(); - const { blobDataSubmission, compressedBlobs, parentShnarf, finalShnarf } = generateBlobDataSubmission(0, 2); - - blobDataSubmission[1].submissionData.firstBlockInData = - blobDataSubmission[1].submissionData.finalBlockInData + 1n; - - const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ - blobDataSubmission, - parentShnarf, - finalShnarf, - ]); - - const { maxFeePerGas, maxPriorityFeePerGas } = await ethers.provider.getFeeData(); - const nonce = await operatorHDSigner.getNonce(); - - const transaction = Transaction.from({ - data: encodedCall, - maxPriorityFeePerGas: maxPriorityFeePerGas!, - maxFeePerGas: maxFeePerGas!, - to: lineaRollupAddress, - chainId: (await ethers.provider.getNetwork()).chainId, - type: 3, - nonce, - value: 0, - gasLimit: 5_000_000, - kzg, - maxFeePerBlobGas: 1n, - blobs: compressedBlobs, - }); - - const signedTx = await operatorHDSigner.signTransaction(transaction); - - await expectRevertWithCustomError( - lineaRollup, - ethers.provider.broadcastTransaction(signedTx), - "DataStartingBlockDoesNotMatch", - [ - blobDataSubmission[0].submissionData.finalBlockInData + 1n, - blobDataSubmission[1].submissionData.firstBlockInData, - ], - ); - }); - - it("Should revert with FirstBlockLessThanOrEqualToLastFinalizedBlock when submitting data with firstBlockInData less than currentL2BlockNumber", async () => { - const operatorHDSigner = getWalletForIndex(2); - - const lineaRollupAddress = await lineaRollup.getAddress(); - const { blobDataSubmission, compressedBlobs, parentShnarf, finalShnarf } = generateBlobDataSubmission(0, 1); - - // Set the currentL2BlockNumber to a value greater than the firstBlockInData - const currentL2BlockNumber = 1_000_000n; - await lineaRollup.setLastFinalizedBlock(currentL2BlockNumber); + blobDataSubmission[emptyDataIndex].snarkHash = generateRandomBytes(32); const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ blobDataSubmission, @@ -939,52 +829,7 @@ describe("Linea Rollup contract", () => { await expectRevertWithCustomError( lineaRollup, ethers.provider.broadcastTransaction(signedTx), - "FirstBlockLessThanOrEqualToLastFinalizedBlock", - [blobDataSubmission[0].submissionData.firstBlockInData, currentL2BlockNumber], - ); - }); - - it("Should revert with FirstBlockGreaterThanFinalBlock when submitting data with firstBlockInData greater than finalBlockInData", async () => { - const operatorHDSigner = getWalletForIndex(2); - - const lineaRollupAddress = await lineaRollup.getAddress(); - const { blobDataSubmission, compressedBlobs, parentShnarf, finalShnarf } = generateBlobDataSubmission(0, 1); - - // Set the firstBlockInData to be greater than the finalBlockInData - blobDataSubmission[0].submissionData.finalBlockInData = - blobDataSubmission[0].submissionData.firstBlockInData - 1n; - - const encodedCall = lineaRollup.interface.encodeFunctionData("submitBlobs", [ - blobDataSubmission, - parentShnarf, - finalShnarf, - ]); - - const { maxFeePerGas, maxPriorityFeePerGas } = await ethers.provider.getFeeData(); - const nonce = await operatorHDSigner.getNonce(); - - const transaction = Transaction.from({ - data: encodedCall, - maxPriorityFeePerGas: maxPriorityFeePerGas!, - maxFeePerGas: maxFeePerGas!, - to: lineaRollupAddress, - chainId: (await ethers.provider.getNetwork()).chainId, - type: 3, - nonce, - value: 0, - gasLimit: 5_000_000, - kzg, - maxFeePerBlobGas: 1n, - blobs: compressedBlobs, - }); - - const signedTx = await operatorHDSigner.signTransaction(transaction); - - await expectRevertWithCustomError( - lineaRollup, - ethers.provider.broadcastTransaction(signedTx), - "FirstBlockGreaterThanFinalBlock", - [blobDataSubmission[0].submissionData.firstBlockInData, blobDataSubmission[0].submissionData.finalBlockInData], + "PointEvaluationFailed", ); }); @@ -1183,7 +1028,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: blobAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(blobAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(blobAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(blobAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(blobAggregatedProof1To155.finalBlockNumber), parentStateRootHash: blobAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(blobAggregatedProof1To155.finalTimestamp), l2MerkleRoots: blobAggregatedProof1To155.l2MerkleRoots, @@ -1221,7 +1066,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: blobAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(blobAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(blobAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(blobAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(blobAggregatedProof1To155.finalBlockNumber), parentStateRootHash: blobAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(blobAggregatedProof1To155.finalTimestamp), l2MerkleRoots: blobAggregatedProof1To155.l2MerkleRoots, @@ -1272,7 +1117,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: blobAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(blobAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(blobAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(blobAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(blobAggregatedProof1To155.finalBlockNumber), parentStateRootHash: blobAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(blobAggregatedProof1To155.finalTimestamp), l2MerkleRoots: blobAggregatedProof1To155.l2MerkleRoots, @@ -1339,7 +1184,7 @@ describe("Linea Rollup contract", () => { }); describe("With and without submission data", () => { - it("Should revert if _finalizationData.finalBlockNumber is less than or equal to currentL2BlockNumber", async () => { + it("Should revert if _finalizationData.endBlockNumber is less than or equal to currentL2BlockNumber", async () => { await lineaRollup.setLastFinalizedBlock(10_000_000); const finalizationData = await generateFinalizationData(); @@ -1358,7 +1203,7 @@ describe("Linea Rollup contract", () => { lineaRollup, finalizeCall, "FinalBlockNumberLessThanOrEqualToLastFinalizedBlock", - [finalizationData.finalBlockInData, 10_000_000], + [finalizationData.endBlockNumber, 10_000_000], ); }); @@ -1393,9 +1238,6 @@ describe("Linea Rollup contract", () => { const parentStateRootHash = await lineaRollup.stateRootHashes(lastFinalizedBlockNumber); finalizationData.parentStateRootHash = parentStateRootHash; - const currentFinalizedShnarf = await lineaRollup.currentFinalizedShnarf(); - finalizationData.lastFinalizedShnarf = currentFinalizedShnarf; - const proof = calldataAggregatedProof1To155.aggregatedProof; const finalizeCall = lineaRollup @@ -1446,7 +1288,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calculateRollingHash(HASH_ZERO, messageHash), l1RollingHashMessageNumber: 10n, lastFinalizedTimestamp: DEFAULT_LAST_FINALIZED_TIMESTAMP, - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(calldataAggregatedProof1To155.finalTimestamp), l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -1507,7 +1349,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calculateRollingHash(HASH_ZERO, messageHash), l1RollingHashMessageNumber: 10n, lastFinalizedTimestamp: DEFAULT_LAST_FINALIZED_TIMESTAMP, - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(new Date(new Date().setHours(new Date().getHours() + 2)).getTime()), // Set to 2 hours in the future l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -1551,7 +1393,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calculateRollingHash(HASH_ZERO, messageHash), l1RollingHashMessageNumber: 10n, lastFinalizedTimestamp: DEFAULT_LAST_FINALIZED_TIMESTAMP, - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(calldataAggregatedProof1To155.finalTimestamp), l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -1691,7 +1533,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calldataAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(calldataAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(calldataAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(calldataAggregatedProof1To155.finalTimestamp), l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -1729,7 +1571,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calldataAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(calldataAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(calldataAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(calldataAggregatedProof1To155.finalTimestamp), l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -1770,7 +1612,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calldataAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(calldataAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(calldataAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(calldataAggregatedProof1To155.finalTimestamp), l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -1809,7 +1651,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: calldataAggregatedProof1To155.l1RollingHash, l1RollingHashMessageNumber: BigInt(calldataAggregatedProof1To155.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(calldataAggregatedProof1To155.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(calldataAggregatedProof1To155.finalBlockNumber), + endBlockNumber: BigInt(calldataAggregatedProof1To155.finalBlockNumber), parentStateRootHash: calldataAggregatedProof1To155.parentStateRootHash, finalTimestamp: BigInt(calldataAggregatedProof1To155.finalTimestamp), l2MerkleRoots: calldataAggregatedProof1To155.l2MerkleRoots, @@ -2055,13 +1897,7 @@ describe("Linea Rollup contract", () => { const receipt = await ethers.provider.getTransactionReceipt(txResponse.hash); - const expectedEventArgs = [ - blobSubmission[0].submissionData.firstBlockInData, - blobSubmission[blobSubmission.length - 1].submissionData.finalBlockInData, - parentShnarf, - finalShnarf, - blobSubmission[blobSubmission.length - 1].submissionData.finalStateRootHash, - ]; + const expectedEventArgs = [parentShnarf, finalShnarf, blobSubmission[blobSubmission.length - 1].finalStateRootHash]; expectEventDirectFromReceiptData(lineaRollup as BaseContract, receipt!, "DataSubmittedV3", expectedEventArgs); } @@ -2080,7 +1916,7 @@ describe("Linea Rollup contract", () => { l1RollingHash: proofData.l1RollingHash, l1RollingHashMessageNumber: BigInt(proofData.l1RollingHashMessageNumber), lastFinalizedTimestamp: BigInt(proofData.parentAggregationLastBlockTimestamp), - finalBlockInData: BigInt(proofData.finalBlockNumber), + endBlockNumber: BigInt(proofData.finalBlockNumber), parentStateRootHash: proofData.parentStateRootHash, finalTimestamp: BigInt(proofData.finalTimestamp), l2MerkleRoots: proofData.l2MerkleRoots, @@ -2100,7 +1936,7 @@ describe("Linea Rollup contract", () => { const eventArgs = [ BigInt(proofData.lastFinalizedBlockNumber) + 1n, - finalizationData.finalBlockInData, + finalizationData.endBlockNumber, proofData.finalShnarf, finalizationData.parentStateRootHash, finalStateRootHash, @@ -2109,13 +1945,13 @@ describe("Linea Rollup contract", () => { await expectEvent(lineaRollup, finalizeCompressedCall, "DataFinalizedV3", eventArgs); const [expectedFinalStateRootHash, lastFinalizedBlockNumber, lastFinalizedState] = await Promise.all([ - lineaRollup.stateRootHashes(finalizationData.finalBlockInData), + lineaRollup.stateRootHashes(finalizationData.endBlockNumber), lineaRollup.currentL2BlockNumber(), lineaRollup.currentFinalizedState(), ]); expect(expectedFinalStateRootHash).to.equal(finalizationData.shnarfData.finalStateRootHash); - expect(lastFinalizedBlockNumber).to.equal(finalizationData.finalBlockInData); + expect(lastFinalizedBlockNumber).to.equal(finalizationData.endBlockNumber); expect(lastFinalizedState).to.equal( generateKeccak256( ["uint256", "bytes32", "uint256"], @@ -2183,7 +2019,9 @@ describe("Linea Rollup contract", () => { // Deploy new implementation const newLineaRollupFactory = await ethers.getContractFactory("contracts/LineaRollup.sol:LineaRollup"); - const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory); + const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory, { + unsafeAllowRenames: true, + }); const upgradedContract = await newLineaRollup.waitForDeployment(); const upgradeCall = upgradedContract.reinitializeLineaRollupV6( @@ -2207,7 +2045,9 @@ describe("Linea Rollup contract", () => { // Deploy new implementation const newLineaRollupFactory = await ethers.getContractFactory("contracts/LineaRollup.sol:LineaRollup"); - const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory); + const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory, { + unsafeAllowRenames: true, + }); const upgradedContract = await newLineaRollup.waitForDeployment(); const upgradeCall = upgradedContract.reinitializeLineaRollupV6( newRoleAddresses, @@ -2224,7 +2064,9 @@ describe("Linea Rollup contract", () => { // Deploy new implementation const newLineaRollupFactory = await ethers.getContractFactory("contracts/LineaRollup.sol:LineaRollup"); - const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory); + const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory, { + unsafeAllowRenames: true, + }); const upgradedContract = await newLineaRollup.waitForDeployment(); await upgradedContract.reinitializeLineaRollupV6( newRoleAddresses, @@ -2246,7 +2088,9 @@ describe("Linea Rollup contract", () => { it("Should revert with ZeroAddressNotAllowed when addressWithRole is zero address in reinitializeLineaRollupV6", async () => { // Deploy new implementation const newLineaRollupFactory = await ethers.getContractFactory("contracts/LineaRollup.sol:LineaRollup"); - const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory); + const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory, { + unsafeAllowRenames: true, + }); const upgradedContract = await newLineaRollup.waitForDeployment(); const roleAddresses = [{ addressWithRole: ZeroAddress, role: DEFAULT_ADMIN_ROLE }, ...newRoleAddresses.slice(1)]; @@ -2265,7 +2109,9 @@ describe("Linea Rollup contract", () => { it("Should set all permissions", async () => { // Deploy new implementation const newLineaRollupFactory = await ethers.getContractFactory("contracts/LineaRollup.sol:LineaRollup"); - const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory); + const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory, { + unsafeAllowRenames: true, + }); const upgradedContract = await newLineaRollup.waitForDeployment(); await upgradedContract.reinitializeLineaRollupV6( @@ -2283,7 +2129,9 @@ describe("Linea Rollup contract", () => { it("Should set all pause types and unpause types in mappings and emit events", async () => { // Deploy new implementation const newLineaRollupFactory = await ethers.getContractFactory("contracts/LineaRollup.sol:LineaRollup"); - const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory); + const newLineaRollup = await upgrades.upgradeProxy(lineaRollup, newLineaRollupFactory, { + unsafeAllowRenames: true, + }); const upgradedContract = await newLineaRollup.waitForDeployment(); const reinitializePromise = upgradedContract.reinitializeLineaRollupV6( diff --git a/contracts/test/common/helpers/dataGeneration.ts b/contracts/test/common/helpers/dataGeneration.ts index 47414a19c..7462ec841 100644 --- a/contracts/test/common/helpers/dataGeneration.ts +++ b/contracts/test/common/helpers/dataGeneration.ts @@ -10,12 +10,10 @@ import { import { FinalizationData, CalldataSubmissionData, - SubmissionAndCompressedData, - BlobSubmissionData, ShnarfData, ParentSubmissionData, ParentAndExpectedShnarf, - SubmissionData, + BlobSubmission, } from "../types"; import { generateRandomBytes, range } from "./general"; import { generateKeccak256 } from "./hashing"; @@ -28,7 +26,7 @@ export const generateL2MessagingBlocksOffsets = (start: number, end: number) => export async function generateFinalizationData(overrides?: Partial): Promise { return { aggregatedProof: generateRandomBytes(928), - finalBlockInData: 99n, + endBlockNumber: 99n, shnarfData: generateParentShnarfData(1), parentStateRootHash: generateRandomBytes(32), lastFinalizedTimestamp: BigInt((await networkTime.latest()) - 2), @@ -48,8 +46,6 @@ export function generateCallDataSubmission(startDataIndex: number, finalDataInde return COMPRESSED_SUBMISSION_DATA.slice(startDataIndex, finalDataIndex).map((data) => { const returnData = { finalStateRootHash: data.finalStateRootHash, - firstBlockInData: BigInt(data.conflationOrder.startingBlockNumber), - finalBlockInData: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), snarkHash: data.snarkHash, compressedData: ethers.hexlify(ethers.decodeBase64(data.compressedData)), }; @@ -61,23 +57,25 @@ export function generateBlobDataSubmission( startDataIndex: number, finalDataIndex: number, isMultiple: boolean = false, -): { blobDataSubmission: BlobSubmissionData[]; compressedBlobs: string[]; parentShnarf: string; finalShnarf: string } { +): { + blobDataSubmission: BlobSubmission[]; + compressedBlobs: string[]; + parentShnarf: string; + finalShnarf: string; +} { const dataSet = isMultiple ? BLOB_SUBMISSION_DATA_MULTIPLE_PROOF : BLOB_SUBMISSION_DATA; const compressedBlobs: string[] = []; const parentShnarf = dataSet[startDataIndex].prevShnarf; const finalShnarf = dataSet[finalDataIndex - 1].expectedShnarf; + const blobDataSubmission = dataSet.slice(startDataIndex, finalDataIndex).map((data) => { compressedBlobs.push(ethers.hexlify(ethers.decodeBase64(data.compressedData))); - const returnData: BlobSubmissionData = { - submissionData: { - finalStateRootHash: data.finalStateRootHash, - firstBlockInData: BigInt(data.conflationOrder.startingBlockNumber), - finalBlockInData: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), - snarkHash: data.snarkHash, - }, + const returnData: BlobSubmission = { dataEvaluationClaim: data.expectedY, kzgCommitment: data.commitment, kzgProof: data.kzgProofContract, + finalStateRootHash: data.finalStateRootHash, + snarkHash: data.snarkHash, }; return returnData; }); @@ -140,15 +138,15 @@ export function generateBlobParentShnarfData(index: number, multiple?: boolean): } export function generateExpectedParentSubmissionHash( - firstBlockInData: bigint, - finalBlockInData: bigint, + firstBlockNumber: bigint, + endBlockNumber: bigint, finalStateRootHash: string, shnarf: string, dataParentHash: string, ): string { return generateKeccak256( ["uint256", "uint256", "bytes32", "bytes32", "bytes32"], - [firstBlockInData, finalBlockInData, finalStateRootHash, shnarf, dataParentHash], + [firstBlockNumber, endBlockNumber, finalStateRootHash, shnarf, dataParentHash], ); } @@ -156,8 +154,8 @@ export function generateParentSubmissionDataForIndex(index: number): ParentSubmi if (index === 0) { return { finalStateRootHash: COMPRESSED_SUBMISSION_DATA[0].parentStateRootHash, - firstBlockInData: 0n, - finalBlockInData: 0n, + firstBlockNumber: 0n, + endBlockNumber: 0n, shnarf: generateKeccak256( ["bytes32", "bytes32", "bytes32", "bytes32", "bytes32"], [HASH_ZERO, HASH_ZERO, COMPRESSED_SUBMISSION_DATA[0].parentStateRootHash, HASH_ZERO, HASH_ZERO], @@ -167,8 +165,8 @@ export function generateParentSubmissionDataForIndex(index: number): ParentSubmi return { finalStateRootHash: COMPRESSED_SUBMISSION_DATA[index - 1].finalStateRootHash, - firstBlockInData: BigInt(COMPRESSED_SUBMISSION_DATA[index - 1].conflationOrder.startingBlockNumber), - finalBlockInData: BigInt(COMPRESSED_SUBMISSION_DATA[index - 1].conflationOrder.upperBoundaries.slice(-1)[0]), + firstBlockNumber: BigInt(COMPRESSED_SUBMISSION_DATA[index - 1].conflationOrder.startingBlockNumber), + endBlockNumber: BigInt(COMPRESSED_SUBMISSION_DATA[index - 1].conflationOrder.upperBoundaries.slice(-1)[0]), shnarf: COMPRESSED_SUBMISSION_DATA[index - 1].expectedShnarf, }; } @@ -191,8 +189,8 @@ export function generateParentSubmissionDataForIndexForMultiple(index: number): if (index === 0) { return { finalStateRootHash: COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[0].parentStateRootHash, - firstBlockInData: 0n, - finalBlockInData: 0n, + firstBlockNumber: 0n, + endBlockNumber: 0n, shnarf: generateKeccak256( ["bytes32", "bytes32", "bytes32", "bytes32", "bytes32"], [HASH_ZERO, HASH_ZERO, COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[0].parentStateRootHash, HASH_ZERO, HASH_ZERO], @@ -201,8 +199,8 @@ export function generateParentSubmissionDataForIndexForMultiple(index: number): } return { finalStateRootHash: COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[index - 1].finalStateRootHash, - firstBlockInData: BigInt(COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[index - 1].conflationOrder.startingBlockNumber), - finalBlockInData: BigInt( + firstBlockNumber: BigInt(COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[index - 1].conflationOrder.startingBlockNumber), + endBlockNumber: BigInt( COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[index - 1].conflationOrder.upperBoundaries.slice(-1)[0], ), shnarf: COMPRESSED_SUBMISSION_DATA_MULTIPLE_PROOF[index - 1].expectedShnarf, @@ -216,8 +214,8 @@ export function generateSubmissionData(startDataIndex: number, finalDataIndex: n parentStateRootHash: data.parentStateRootHash, dataParentHash: data.parentDataHash, finalStateRootHash: data.finalStateRootHash, - firstBlockInData: BigInt(data.conflationOrder.startingBlockNumber), - finalBlockInData: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), + firstBlockNumber: BigInt(data.conflationOrder.startingBlockNumber), + endBlockNumber: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), snarkHash: data.snarkHash, }, compressedData: ethers.hexlify(ethers.decodeBase64(data.compressedData)), @@ -236,8 +234,8 @@ export function generateSubmissionDataMultipleProofs( parentStateRootHash: data.parentStateRootHash, dataParentHash: data.parentDataHash, finalStateRootHash: data.finalStateRootHash, - firstBlockInData: BigInt(data.conflationOrder.startingBlockNumber), - finalBlockInData: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), + firstBlockNumber: BigInt(data.conflationOrder.startingBlockNumber), + endBlockNumber: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), snarkHash: data.snarkHash, }, compressedData: ethers.hexlify(ethers.decodeBase64(data.compressedData)), @@ -254,8 +252,8 @@ export function generateCallDataSubmissionMultipleProofs( parentStateRootHash: data.parentStateRootHash, dataParentHash: data.parentDataHash, finalStateRootHash: data.finalStateRootHash, - firstBlockInData: BigInt(data.conflationOrder.startingBlockNumber), - finalBlockInData: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), + firstBlockNumber: BigInt(data.conflationOrder.startingBlockNumber), + endBlockNumber: BigInt(data.conflationOrder.upperBoundaries.slice(-1)[0]), snarkHash: data.snarkHash, compressedData: ethers.hexlify(ethers.decodeBase64(data.compressedData)), parentShnarf: data.prevShnarf, @@ -274,8 +272,8 @@ export function generateSubmissionDataFromJSON( parentStateRootHash: parsedJSONData.parentStateRootHash, dataParentHash: parsedJSONData.parentDataHash, finalStateRootHash: parsedJSONData.finalStateRootHash, - firstBlockInData: BigInt(startingBlockNumber), - finalBlockInData: BigInt(endingBlockNumber), + firstBlockNumber: BigInt(startingBlockNumber), + endBlockNumber: BigInt(endingBlockNumber), snarkHash: parsedJSONData.snarkHash, compressedData: ethers.hexlify(ethers.decodeBase64(parsedJSONData.compressedData)), }; @@ -289,7 +287,7 @@ export function generateFinalizationDataFromJSON(parsedJSONData: any): Finalizat const { aggregatedProverVersion, aggregatedVerifierIndex, aggregatedProofPublicInput, ...data } = parsedJSONData; return { ...data, - finalBlockNumber: BigInt(data.finalBlockNumber), + endBlockNumber: BigInt(data.endBlockNumber), l1RollingHashMessageNumber: BigInt(data.l1RollingHashMessageNumber), l2MerkleTreesDepth: BigInt(data.l2MerkleTreesDepth), l2MessagingBlocksOffsets: data.l2MessagingBlocksOffsets, diff --git a/contracts/test/common/types.ts b/contracts/test/common/types.ts index 2bd08eb78..a33a30abd 100644 --- a/contracts/test/common/types.ts +++ b/contracts/test/common/types.ts @@ -33,24 +33,18 @@ export type DebugData = { finalHash: string; }; -export type SubmissionData = { - finalStateRootHash: string; - firstBlockInData: bigint; - finalBlockInData: bigint; - snarkHash: string; -}; - -export type BlobSubmissionData = { - submissionData: SubmissionData; +export type BlobSubmission = { dataEvaluationClaim: string; kzgCommitment: string; kzgProof: string; + finalStateRootHash: string; + snarkHash: string; }; export type ParentSubmissionData = { finalStateRootHash: string; - firstBlockInData: bigint; - finalBlockInData: bigint; + firstBlockNumber: bigint; + endBlockNumber: bigint; shnarf: string; }; @@ -67,18 +61,15 @@ export type ShnarfData = { dataEvaluationClaim: string; }; -export type CalldataSubmissionData = SubmissionData & { - compressedData: string; -}; - -export type SubmissionAndCompressedData = { - submissionData: SubmissionData; +export type CalldataSubmissionData = { + finalStateRootHash: string; + snarkHash: string; compressedData: string; }; export type FinalizationData = { aggregatedProof: string; - finalBlockInData: bigint; + endBlockNumber: bigint; shnarfData: ShnarfData; parentStateRootHash: string; lastFinalizedTimestamp: bigint; diff --git a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt index 56e43b539..b6d09b729 100644 --- a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt +++ b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/app/config/CoordinatorConfigTest.kt @@ -167,7 +167,7 @@ class CoordinatorConfigTest { "b1504a5f" to "BlobSubmissionDataIsMissing", "c0e41e1d" to "EmptyBlobDataAtIndex", "fb4cd6ef" to "FinalBlockDoesNotMatchShnarfFinalBlock", - "2526F108 " to "ShnarfAndFinalBlockNumberLengthsMismatched", + "2526F108" to "ShnarfAndFinalBlockNumberLengthsMismatched", "d3664fb3" to "FinalShnarfWrong", "4e686675" to "L2MerkleRootDoesNotExist", "e5d14425" to "L2MerkleRootAlreadyAnchored", @@ -180,6 +180,7 @@ class CoordinatorConfigTest { "b015579f" to "IsNotPaused", "3b174434" to "MessageHashesListLengthHigherThanOneHundred", "ca389c44" to "InvalidProofOrProofVerificationRanOutOfGas", + "42ab979d" to "ParentBlobNotSubmitted", // L2 Message Service "6446cc9c" to "MessageHashesListLengthIsZero", "d39e75f9" to "L1MessageNumberSynchronizationWrong", From 5fad7f793cc10f3030b130d4127e40ef284ecfcf Mon Sep 17 00:00:00 2001 From: The Dark Jester Date: Sun, 20 Oct 2024 08:47:24 -0700 Subject: [PATCH 6/6] [Fix] Set default admin for token bridge on reinitialization (#211) * set default admin for token bridge on reinit * Use more applicable naming for admin --- .../contracts/tokenBridge/TokenBridge.sol | 11 ++++++- contracts/test/tokenBridge/TokenBridge.ts | 31 +++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/tokenBridge/TokenBridge.sol b/contracts/contracts/tokenBridge/TokenBridge.sol index be61b4f90..65428fd63 100644 --- a/contracts/contracts/tokenBridge/TokenBridge.sol +++ b/contracts/contracts/tokenBridge/TokenBridge.sol @@ -154,21 +154,30 @@ contract TokenBridge is /** * @notice Sets permissions for a list of addresses and their roles as well as initialises the PauseManager pauseType:role mappings. * @dev This function is a reinitializer and can only be called once per version. Should be called using an upgradeAndCall transaction to the ProxyAdmin. + * @param _defaultAdmin The default admin account's address. * @param _roleAddresses The list of addresses and roles to assign permissions to. * @param _pauseTypeRoles The list of pause types to associate with roles. * @param _unpauseTypeRoles The list of unpause types to associate with roles. */ function reinitializePauseTypesAndPermissions( + address _defaultAdmin, RoleAddress[] calldata _roleAddresses, PauseTypeRole[] calldata _pauseTypeRoles, PauseTypeRole[] calldata _unpauseTypeRoles ) external reinitializer(2) { + if (_defaultAdmin == address(0)) { + revert ZeroAddressNotAllowed(); + } + + _grantRole(DEFAULT_ADMIN_ROLE, _defaultAdmin); + assembly { /// @dev Wiping the storage slot 101 of _owner as it is replaced by AccessControl and there is now the ERC165 __gap in its place. - /// @dev Wiping the storage slot 213 of _status as it is replaced by ReentrancyGuardUpgradeable at slot 1. sstore(101, 0) + /// @dev Wiping the storage slot 213 of _status as it is replaced by ReentrancyGuardUpgradeable at slot 1. sstore(213, 0) } + __ReentrancyGuard_init(); __PauseManager_init(_pauseTypeRoles, _unpauseTypeRoles); __Permissions_init(_roleAddresses); diff --git a/contracts/test/tokenBridge/TokenBridge.ts b/contracts/test/tokenBridge/TokenBridge.ts index 7c27dd456..0c268121f 100644 --- a/contracts/test/tokenBridge/TokenBridge.ts +++ b/contracts/test/tokenBridge/TokenBridge.ts @@ -30,6 +30,7 @@ import { expectRevertWithCustomError, expectRevertWithReason, } from "../common/helpers"; +import { DEFAULT_ADMIN_ROLE } from "contracts/common/constants"; const initialUserBalance = BigInt(10 ** 9); const mockName = "L1 DAI"; @@ -905,13 +906,35 @@ describe("TokenBridge", function () { describe("reinitializePauseTypesAndPermissions", function () { it("Should revert with ZeroAddressNotAllowed when addressWithRole is zero address in reinitializePauseTypesAndPermissions", async function () { - const { l1TokenBridge } = await loadFixture(deployContractsFixture); + const { owner, l1TokenBridge } = await loadFixture(deployContractsFixture); const roleAddresses = [{ addressWithRole: ZeroAddress, role: SET_RESERVED_TOKEN_ROLE }]; await expectRevertWithCustomError( l1TokenBridge, - l1TokenBridge.reinitializePauseTypesAndPermissions(roleAddresses, pauseTypeRoles, unpauseTypeRoles), + l1TokenBridge.reinitializePauseTypesAndPermissions( + owner.address, + roleAddresses, + pauseTypeRoles, + unpauseTypeRoles, + ), + "ZeroAddressNotAllowed", + ); + }); + + it("Should revert with ZeroAddressNotAllowed when default admin is zero address", async function () { + const { owner, l1TokenBridge } = await loadFixture(deployContractsFixture); + + const roleAddresses = [{ addressWithRole: owner.address, role: SET_RESERVED_TOKEN_ROLE }]; + + await expectRevertWithCustomError( + l1TokenBridge, + l1TokenBridge.reinitializePauseTypesAndPermissions( + ZeroAddress, //owner is set to zeroaddress + roleAddresses, + pauseTypeRoles, + unpauseTypeRoles, + ), "ZeroAddressNotAllowed", ); }); @@ -971,6 +994,7 @@ describe("TokenBridge", function () { await tokenBridgeV2Implementation.waitForDeployment(); const reinitializeCallData = TokenBridgeV2.interface.encodeFunctionData("reinitializePauseTypesAndPermissions", [ + owner.address, newRoleAddresses, pauseTypeRoles, unpauseTypeRoles, @@ -1062,6 +1086,7 @@ describe("TokenBridge", function () { await tokenBridgeV2Implementation.waitForDeployment(); const reinitializeCallData = TokenBridgeV2.interface.encodeFunctionData("reinitializePauseTypesAndPermissions", [ + owner.address, newRoleAddresses, pauseTypeRoles, unpauseTypeRoles, @@ -1085,6 +1110,8 @@ describe("TokenBridge", function () { for (const { role, addressWithRole } of newRoleAddresses) { expect(await upgradedTokenBridge.hasRole(role, addressWithRole)).to.be.true; } + + expect(await upgradedTokenBridge.hasRole(DEFAULT_ADMIN_ROLE, owner.address)).to.be.true; }); }); });