From 01601f7c063fdd9134882325cef4e323b9aa8b3a Mon Sep 17 00:00:00 2001 From: Pedro Novais <1478752+jpnovais@users.noreply.github.com> Date: Wed, 16 Oct 2024 16:57:58 +0100 Subject: [PATCH 01/12] coordinator: fix integration test smart contract deploy helper (#197) coordinator: fix integration test smart contract deploy helper --- .../MakefileContractDeploymentHelper.kt | 17 +++-- .../MakefileContractDeploymentHelperKtTest.kt | 65 +++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 coordinator/ethereum/test-utils/src/test/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelperKtTest.kt diff --git a/coordinator/ethereum/test-utils/src/main/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelper.kt b/coordinator/ethereum/test-utils/src/main/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelper.kt index 07f80faea..c616b0ea9 100644 --- a/coordinator/ethereum/test-utils/src/main/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelper.kt +++ b/coordinator/ethereum/test-utils/src/main/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelper.kt @@ -74,11 +74,11 @@ fun executeCommand( return futureResult.toSafeFuture() } -private val lineaRollupAddressPattern = Pattern.compile( - "^LineaRollup(?:AlphaV3)? deployed: address=(0x[0-9a-fA-F]{40}) blockNumber=(\\d+)" +internal val lineaRollupAddressPattern = Pattern.compile( + "^LineaRollup(?:.*)? deployed: address=(0x[0-9a-fA-F]{40}) blockNumber=(\\d+)" ) -private val l2MessageServiceAddressPattern = Pattern.compile( - "^L2MessageService deployed: address=(0x[0-9a-fA-F]{40}) blockNumber=(\\d+)" +internal val l2MessageServiceAddressPattern = Pattern.compile( + "^L2MessageService(?:.*)? deployed: address=(0x[0-9a-fA-F]{40}) blockNumber=(\\d+)" ) data class DeployedContract( @@ -91,7 +91,14 @@ fun getDeployedAddress( addressPattern: Pattern ): DeployedContract { val lines = commandResult.stdOut.toList().asReversed() - val matcher: Matcher? = lines + return getDeployedAddress(lines, addressPattern) +} + +fun getDeployedAddress( + cmdStdoutLines: List, + addressPattern: Pattern +): DeployedContract { + val matcher: Matcher? = cmdStdoutLines .firstOrNull { line -> addressPattern.matcher(line).find() } ?.let { addressPattern.matcher(it).also { it.find() } } diff --git a/coordinator/ethereum/test-utils/src/test/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelperKtTest.kt b/coordinator/ethereum/test-utils/src/test/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelperKtTest.kt new file mode 100644 index 000000000..62f8ed1f1 --- /dev/null +++ b/coordinator/ethereum/test-utils/src/test/kotlin/net/consensys/zkevm/ethereum/MakefileContractDeploymentHelperKtTest.kt @@ -0,0 +1,65 @@ +package net.consensys.zkevm.ethereum + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class MakefileContractDeploymentHelperKtTest { + + @Test + fun getDeployedAddress_messageService() { + assertThat( + getDeployedAddress( + listOf( + "L2MessageService artifact has been deployed in 1.2659626659999998s ", + "L2MessageService deployed: address=0xFE48d65B84AA0E23594Fd585c11cAD831F90AcB6 blockNumber=8", + "" + ), + l2MessageServiceAddressPattern + ) + ).isEqualTo( + DeployedContract("0xFE48d65B84AA0E23594Fd585c11cAD831F90AcB6", 8) + ) + + assertThat( + getDeployedAddress( + listOf( + "L2MessageServiceV1.2.3 artifact has been deployed in 1.2659626659999998s ", + "L2MessageServiceV1.2.3 deployed: address=0xFE48d65B84AA0E23594Fd585c11cAD831F90AcB6 blockNumber=8", + "" + ), + l2MessageServiceAddressPattern + ) + ).isEqualTo( + DeployedContract("0xFE48d65B84AA0E23594Fd585c11cAD831F90AcB6", 8) + ) + } + + @Test + fun getDeployedAddress_LineaRollup() { + assertThat( + getDeployedAddress( + listOf( + "LineaRollup artifact has been deployed in 1.855172125s ", + "LineaRollup deployed: address=0x8613180dF1485B8b87DEE3BCf31896659eb1a092 blockNumber=1414", + "" + ), + lineaRollupAddressPattern + ) + ).isEqualTo( + DeployedContract("0x8613180dF1485B8b87DEE3BCf31896659eb1a092", 1414) + ) + + assertThat( + getDeployedAddress( + listOf( + "LineaRollupV5.2.1 artifact has been deployed in 1.855172125s ", + "LineaRollupV5.2.1 deployed: address=0x8613180dF1485B8b87DEE3BCf31896659eb1a092 blockNumber=1414", + "" + ), + lineaRollupAddressPattern + ) + ).isEqualTo( + DeployedContract("0x8613180dF1485B8b87DEE3BCf31896659eb1a092", 1414) + ) + } +} From 946b0ef43c71616abffbb989186a5786442dbdae Mon Sep 17 00:00:00 2001 From: Nadeem Bhati Date: Wed, 16 Oct 2024 21:31:15 +0530 Subject: [PATCH 02/12] enhance TracesFilesManager test coverage (#137) * test: add edge case coverage for TracesFilesManager * remove unused aftereach import --------- Co-authored-by: Victorien Gauch <85494462+VGau@users.noreply.github.com> --- .../blockcreation/TracesFilesManagerTest.kt | 95 ++++++++++++++----- 1 file changed, 72 insertions(+), 23 deletions(-) diff --git a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt index c6163c147..e3ea8aa11 100644 --- a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt +++ b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt @@ -93,14 +93,14 @@ class TracesFilesManagerTest { } @Test - fun `waitRawTracesGenerationOf - waits until traces file is found`() { - // write file with wrong extension - val inprogressFile = - tracesDir.resolve(Path.of("1-${block1Hash.toHexString()}.inprogress")).createFile() + fun `waitRawTracesGenerationOf waits until traces file is found`() { + val inprogressFile = tracesDir + .resolve(Path.of("1-${block1Hash.toHexString()}.inprogress")) + .createFile() assertThat(inprogressFile).exists() val future = tracesFilesManager.waitRawTracesGenerationOf(1uL, block1Hash) - vertx.setTimer((config.tracesGenerationTimeout.inWholeMilliseconds / 2)) { + vertx.setTimer(config.tracesGenerationTimeout.inWholeMilliseconds / 2) { Files.createFile(block1TracesFile) } @@ -108,22 +108,22 @@ class TracesFilesManagerTest { } @RepeatedTest(10) - fun `waitRawTracesGenerationOf - returns error after timeout`() { + fun `waitRawTracesGenerationOf returns error after timeout`() { val future = tracesFilesManager.waitRawTracesGenerationOf(2uL, block2Hash1) val exception = assertThrows { future.get() } assertThat(exception.cause).isInstanceOf(FileNotFoundException::class.java) - assertThat(exception.message).matches(".* File matching '2-$block2Hash1.* not found .*") + assertThat(exception.message) + .matches(".* File matching '2-$block2Hash1.* not found .*") } @Test - fun `cleanNonCanonicalSiblingsByHeight - returns error when file to keep is not found`() { + fun `cleanNonCanonicalSiblingsByHeight returns error when file to keep is not found`() { val future = tracesFilesManager.cleanNonCanonicalSiblingsByHeight(1uL, block1Hash) - assertThat(future.get()).isEmpty() } @Test - fun `cleanNonCanonicalSiblingsByHeight - removes found siblings`() { + fun `cleanNonCanonicalSiblingsByHeight removes found siblings`() { Files.createFile(block2TracesFile1) Files.createFile(block2TracesFile2) Files.createFile(block20TracesFile) @@ -138,17 +138,66 @@ class TracesFilesManagerTest { assertThat(block20TracesFile).exists() } - // @Test - // @Disabled("This feature is not necessary anymore. Just keeping the test in case we need to revert") - // fun `waitRawTracesGenerationOf - cleans non canonical siblings`() { - // Files.createFile(block2TracesFile1) - // Files.createFile(block2TracesFile2) - // assertThat(block2TracesFile1).exists() - // assertThat(block2TracesFile2).exists() - // - // tracesFilesManager.waitRawTracesGenerationOf(UInt64.valueOf(2), block2Hash1).get() - // - // assertThat(block2TracesFile1).exists() - // assertThat(block2TracesFile2).doesNotExist() - // } + @Test + fun `initialization fails when nonCanonicalTracesDir doesn't exist and creation is disabled`() { + val configWithoutDirCreation = config.copy( + createNonCanonicalTracesDirIfDoesNotExist = false + ) + Files.delete(nonCanonicalBlocksTracesDir) + + assertThrows { + TracesFilesManager(vertx, configWithoutDirCreation) + } + } + + @Test + fun `initialization creates nonCanonicalTracesDir when it doesn't exist and creation is enabled`() { + Files.delete(nonCanonicalBlocksTracesDir) + + TracesFilesManager(vertx, config) + + assertThat(nonCanonicalBlocksTracesDir).exists() + } + + @Test + fun `cleanNonCanonicalSiblingsByHeight moves files to nonCanonicalTracesDir`() { + Files.createFile(block2TracesFile1) + Files.createFile(block2TracesFile2) + + tracesFilesManager.cleanNonCanonicalSiblingsByHeight(2uL, block2Hash1).get() + + val movedFile = nonCanonicalBlocksTracesDir.resolve(block2TracesFile2.fileName) + assertThat(movedFile).exists() + assertThat(block2TracesFile2).doesNotExist() + } + + @Test + fun `cleanNonCanonicalSiblingsByHeight handles case sensitivity correctly`() { + val mixedCaseHash = block2Hash1.toHexString().uppercase() + val tracesFileName = TracesFiles.rawTracesFileNameSupplierV1( + 2uL, + Bytes32.fromHexString(mixedCaseHash), + tracesVersion, + tracesFileExtension + ) + val mixedCaseFile = tracesDir.resolve(Path.of(tracesFileName)) + Files.createFile(mixedCaseFile) + Files.createFile(block2TracesFile2) + + tracesFilesManager.cleanNonCanonicalSiblingsByHeight(2uL, block2Hash1).get() + + assertThat(mixedCaseFile).exists() + assertThat(block2TracesFile2).doesNotExist() + } + + @Test + fun `waitRawTracesGenerationOf handles extremely short polling interval`() { + val configWithShortPolling = config.copy(pollingInterval = 1.milliseconds) + val manager = TracesFilesManager(vertx, configWithShortPolling) + + val future = manager.waitRawTracesGenerationOf(1uL, block1Hash) + vertx.setTimer(50) { Files.createFile(block1TracesFile) } + + assertThat(future.get()).endsWith(block1TracesFile.toString()) + } } From f53e2d11d48d09a0d07075d7fc5c52504dd05dd7 Mon Sep 17 00:00:00 2001 From: Pedro Novais <1478752+jpnovais@users.noreply.github.com> Date: Wed, 16 Oct 2024 18:59:10 +0100 Subject: [PATCH 03/12] fix coordinator local overrides file (#199) * fix coordinator local overrides file * coordinator: removes broken test --- ...oordinator-local-dev.config.overrides.toml | 15 +++++-------- .../blockcreation/TracesFilesManagerTest.kt | 22 ------------------- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/config/coordinator/coordinator-local-dev.config.overrides.toml b/config/coordinator/coordinator-local-dev.config.overrides.toml index 9a0a301fe..30ab4b87f 100644 --- a/config/coordinator/coordinator-local-dev.config.overrides.toml +++ b/config/coordinator/coordinator-local-dev.config.overrides.toml @@ -38,15 +38,12 @@ non-canonical-raw-traces-directory = "tmp/local/traces/raw-non-canonical" [state-manager] endpoints=["http://127.0.0.1:8998/"] -[dynamic-gas-price-service] -geth-gas-price-update-recipients=[ - "http://127.0.0.1:8645", - "http://127.0.0.1:8845" -] -besu-gas-price-update-recipients=[ - "http://127.0.0.1:8545/" -] -extra-data-update-recipient="http://127.0.0.1:8545/" +[l2-network-gas-pricing.extra-data-pricing-propagation] +extra-data-update-recipient = "http://127.0.0.1:8545/" + +[l2-network-gas-pricing.json-rpc-pricing-propagation] +geth-gas-price-update-recipients = ["http://127.0.0.1:8845"] +besu-gas-price-update-recipients = [] [l1] rpc-endpoint="http://127.0.0.1:8445" diff --git a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt index e3ea8aa11..e74982bac 100644 --- a/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt +++ b/coordinator/app/src/test/kotlin/net/consensys/zkevm/coordinator/blockcreation/TracesFilesManagerTest.kt @@ -15,7 +15,6 @@ import java.nio.file.Files import java.nio.file.Path import java.util.concurrent.ExecutionException import kotlin.io.path.createFile -import kotlin.io.path.exists import kotlin.time.Duration.Companion.milliseconds class TracesFilesManagerTest { @@ -31,8 +30,6 @@ class TracesFilesManagerTest { Bytes32.fromHexString("0x00000000000000000000000000000000000000000000000000000000000000a1") private val block2Hash2 = Bytes32.fromHexString("0x00000000000000000000000000000000000000000000000000000000000000a2") - private val block20Hash = - Bytes32.fromHexString("0x0000000000000000000000000000000000000000000000000000000000000001") private lateinit var block1TracesFile: Path private lateinit var block2TracesFile1: Path private lateinit var block2TracesFile2: Path @@ -171,25 +168,6 @@ class TracesFilesManagerTest { assertThat(block2TracesFile2).doesNotExist() } - @Test - fun `cleanNonCanonicalSiblingsByHeight handles case sensitivity correctly`() { - val mixedCaseHash = block2Hash1.toHexString().uppercase() - val tracesFileName = TracesFiles.rawTracesFileNameSupplierV1( - 2uL, - Bytes32.fromHexString(mixedCaseHash), - tracesVersion, - tracesFileExtension - ) - val mixedCaseFile = tracesDir.resolve(Path.of(tracesFileName)) - Files.createFile(mixedCaseFile) - Files.createFile(block2TracesFile2) - - tracesFilesManager.cleanNonCanonicalSiblingsByHeight(2uL, block2Hash1).get() - - assertThat(mixedCaseFile).exists() - assertThat(block2TracesFile2).doesNotExist() - } - @Test fun `waitRawTracesGenerationOf handles extremely short polling interval`() { val configWithShortPolling = config.copy(pollingInterval = 1.milliseconds) From 82ae1432664ad3bef77fc9d4223d8ecee565f156 Mon Sep 17 00:00:00 2001 From: Azam Soleimanian <49027816+Soleimani193@users.noreply.github.com> Date: Thu, 17 Oct 2024 12:11:34 +0200 Subject: [PATCH 04/12] fixed multi column (#171) --- prover/protocol/compiler/lookup/compiler.go | 24 ++++--- .../protocol/compiler/lookup/compiler_test.go | 64 ++++++++++++++++++- .../compiler/lookup/conditional_test.go | 10 +-- prover/protocol/compiler/lookup/prover.go | 3 +- prover/protocol/compiler/lookup/utils.go | 2 +- prover/protocol/compiler/lookup/verifier.go | 6 +- prover/protocol/compiler/lookup/z_packing.go | 25 ++++---- .../selfrecursion/selfrecursion_test.go | 6 +- .../compiler/specialqueries/range_test.go | 37 +++++++++++ prover/protocol/wizardutils/evaluation.go | 11 +++- 10 files changed, 149 insertions(+), 39 deletions(-) create mode 100644 prover/protocol/compiler/specialqueries/range_test.go diff --git a/prover/protocol/compiler/lookup/compiler.go b/prover/protocol/compiler/lookup/compiler.go index 2bb571bc8..6b4f8fea6 100644 --- a/prover/protocol/compiler/lookup/compiler.go +++ b/prover/protocol/compiler/lookup/compiler.go @@ -28,7 +28,7 @@ func CompileLogDerivative(comp *wizard.CompiledIOP) { var ( mainLookupCtx = captureLookupTables(comp) - lastRound = comp.NumRounds() + lastRound = comp.NumRounds() - 1 proverActions = make([]proverTaskAtRound, comp.NumRounds()+1) // zCatalog stores a mapping (round, size) into ZCtx and helps finding // which Z context should be used to handle a part of a given permutation @@ -101,9 +101,11 @@ func CompileLogDerivative(comp *wizard.CompiledIOP) { // z-packing compile zC.compile(comp) // entry[0]:round, entry[1]: size + // the round that Gamma was registered. round := entry[0] - proverActions[round+1].pushZAssignment(zAssignmentTask(*zC)) + proverActions[round].pushZAssignment(zAssignmentTask(*zC)) va.ZOpenings = append(va.ZOpenings, zC.ZOpenings...) + va.Name = zC.Name } for round := range proverActions { @@ -209,6 +211,8 @@ func captureLookupTables(comp *wizard.CompiledIOP) mainLookupCtx { // - (3) The verifier makes a `Local` query : $(\Sigma_T)[0] = \frac{M_0}{T_0 + \gamma}$ // - (4) **(For all k)** The verifier makes a `Global` query : $\left((\Sigma_{S,k})[i] - (\Sigma_{S,k})[i-1]\right)(S_{k,i} + \gamma) = 1$ // - (5) The verier makes a `Global` query : $\left((\Sigma_T)[i] - (\Sigma_T)[i-1]\right)(T_i + \gamma) = M_i$ + +// here we are looking up set of columns S in a single column T func compileLookupTable( comp *wizard.CompiledIOP, round int, @@ -228,7 +232,7 @@ func compileLookupTable( var ( // isMultiColumn indicates whether the lookup table (and thus the // checked tables) have the same number of - isMultiColumn = len(lookupTable) > 1 + isMultiColumn = len(lookupTable[0]) > 1 ) if !isMultiColumn { @@ -285,7 +289,7 @@ func compileLookupTable( func (stc *singleTableCtx) pushToZCatalog(zCatalog map[[2]int]*zCtx) { var ( - round = stc.M[0].Round() + round = stc.Gamma.Round ) // tableCtx push to -> zCtx @@ -298,6 +302,7 @@ func (stc *singleTableCtx) pushToZCatalog(zCatalog map[[2]int]*zCtx) { zCatalog[key] = &zCtx{ Size: size, Round: round, + Name: stc.TableName, } } @@ -307,14 +312,14 @@ func (stc *singleTableCtx) pushToZCatalog(zCatalog map[[2]int]*zCtx) { } // Process the S columns - for frag := range stc.S { + for table := range stc.S { var ( - _, _, size = wizardutils.AsExpr(stc.S[frag]) + _, _, size = wizardutils.AsExpr(stc.S[table]) sFilter = symbolic.NewConstant(1) ) - if stc.SFilters[frag] != nil { - sFilter = symbolic.NewVariable(stc.SFilters[frag]) + if stc.SFilters[table] != nil { + sFilter = symbolic.NewVariable(stc.SFilters[table]) } key := [2]int{round, size} @@ -322,11 +327,12 @@ func (stc *singleTableCtx) pushToZCatalog(zCatalog map[[2]int]*zCtx) { zCatalog[key] = &zCtx{ Size: size, Round: round, + Name: stc.TableName, } } zCtxEntry := zCatalog[key] zCtxEntry.SigmaNumerator = append(zCtxEntry.SigmaNumerator, sFilter) - zCtxEntry.SigmaDenominator = append(zCtxEntry.SigmaDenominator, symbolic.Add(stc.Gamma, stc.S[frag])) + zCtxEntry.SigmaDenominator = append(zCtxEntry.SigmaDenominator, symbolic.Add(stc.Gamma, stc.S[table])) } } diff --git a/prover/protocol/compiler/lookup/compiler_test.go b/prover/protocol/compiler/lookup/compiler_test.go index 638329308..867836b59 100644 --- a/prover/protocol/compiler/lookup/compiler_test.go +++ b/prover/protocol/compiler/lookup/compiler_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/consensys/linea-monorepo/prover/maths/common/smartvectors" + "github.com/consensys/linea-monorepo/prover/maths/field" + "github.com/consensys/linea-monorepo/prover/protocol/coin" "github.com/consensys/linea-monorepo/prover/protocol/compiler/dummy" "github.com/consensys/linea-monorepo/prover/protocol/ifaces" "github.com/consensys/linea-monorepo/prover/protocol/wizard" @@ -78,9 +80,11 @@ func TestLogDerivativeLookupManyChecksOneTable(t *testing.T) { define := func(b *wizard.Builder) { cola := b.RegisterCommit("S", sizeA) cola2 := b.RegisterCommit("S2", sizeA) + cola3 := b.RegisterCommit("S3", sizeA) colb := b.RegisterCommit("T", sizeB) b.Inclusion("LOOKUP", []ifaces.Column{colb}, []ifaces.Column{cola}) b.Inclusion("LOOKUP2", []ifaces.Column{colb}, []ifaces.Column{cola2}) + b.Inclusion("LOOKUP3", []ifaces.Column{colb}, []ifaces.Column{cola3}) } prover := func(run *wizard.ProverRuntime) { @@ -88,10 +92,12 @@ func TestLogDerivativeLookupManyChecksOneTable(t *testing.T) { // assign a and b cola := smartvectors.ForTest(1, 1, 1, 2, 3, 0, 0, 1, 1, 1, 1, 2, 3, 0, 0, 1) cola2 := smartvectors.ForTest(2, 2, 2, 1, 0, 3, 3, 2, 2, 2, 2, 1, 0, 3, 3, 2) + cola3 := smartvectors.ForTest(2, 2, 2, 1, 0, 3, 3, 2, 2, 2, 2, 1, 0, 3, 3, 3) colb := smartvectors.ForTest(0, 1, 2, 3) // m expected to be run.AssignColumn("S", cola) run.AssignColumn("S2", cola2) + run.AssignColumn("S3", cola3) run.AssignColumn("T", colb) } @@ -99,7 +105,7 @@ func TestLogDerivativeLookupManyChecksOneTable(t *testing.T) { proof := wizard.Prove(comp, prover) // m should be - expectedM := smartvectors.ForTest(6, 10, 10, 6) + expectedM := smartvectors.ForTest(8, 12, 17, 11) t.Logf("all columns = %v", runtime.Columns.ListAllKeys()) actualM := runtime.GetColumn("TABLE_T_0_LOGDERIVATIVE_M") @@ -155,7 +161,7 @@ func TestLogDerivativeLookupOneXor(t *testing.T) { // m should be expectedM := smartvectors.ForTest(0, 0, 0, 1, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0) t.Logf("all columns = %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_XOR_TABLE_X_XOR_TABLE_XXORY_XOR_TABLE_Y_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_XOR_TABLE_X,XOR_TABLE_XXORY,XOR_TABLE_Y_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") @@ -222,7 +228,7 @@ func TestLogDerivativeLookupMultiXor(t *testing.T) { // m should be expectedM := smartvectors.ForTest(1, 1, 1, 2, 2, 1, 1, 1, 0, 1, 0, 0, 0, 0, 1, 0) t.Logf("all column names = %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_XOR_TABLE_X_XOR_TABLE_XXORY_XOR_TABLE_Y_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_XOR_TABLE_X,XOR_TABLE_XXORY,XOR_TABLE_Y_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") @@ -230,6 +236,58 @@ func TestLogDerivativeLookupMultiXor(t *testing.T) { require.NoError(t, err) } +func TestLogDerivativeLookupRandomLinComb(t *testing.T) { + + var sizeA, sizeB int = 16, 8 + var col1, col2 ifaces.Column + define := func(b *wizard.Builder) { + col1 = b.RegisterPrecomputed("P1", smartvectors.ForTest(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)) + col2 = b.RegisterPrecomputed("P2", smartvectors.ForTest(12, 6, 8, 0, 3, 12, 13, 23, 17, 9, 8, 7, 6, 5, 4, 3)) + colI := b.RegisterPrecomputed("I", smartvectors.ForTest(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15)) + + _ = b.RegisterRandomCoin("COIN", coin.Field) + + uCol := b.InsertProof(1, "LC", sizeA) + + _ = b.RegisterRandomCoin("COIN1", coin.Field) + + colQ := b.RegisterCommit("Q", sizeB) + uChosen := b.RegisterCommit("UChosen", sizeB) + + // multi-col query + b.Inclusion("LOOKUP", []ifaces.Column{colI, uCol}, []ifaces.Column{colQ, uChosen}) + } + + prover := func(run *wizard.ProverRuntime) { + // assign a and b + + coin := run.GetRandomCoinField("COIN") + + a := col1.GetColAssignment(run) + b := col2.GetColAssignment(run) + lc := smartvectors.PolyEval([]smartvectors.SmartVector{a, b}, coin) + + run.AssignColumn("LC", lc) + + run.GetRandomCoinField("COIN1") + + colQ := smartvectors.ForTest(0, 1, 2, 3, 4, 5, 6, 7) + run.AssignColumn("Q", colQ) + + colQFr := colQ.IntoRegVecSaveAlloc() + var t []field.Element + for _, q := range colQFr { + t = append(t, lc.Get(int(q.Uint64()))) + } + run.AssignColumn("UChosen", smartvectors.NewRegular(t)) + } + + comp := wizard.Compile(define, CompileLogDerivative, dummy.Compile) + proof := wizard.Prove(comp, prover) + err := wizard.Verify(comp, proof) + require.NoError(t, err) +} + func BenchmarkLogDeriveLookupMultiXor(b *testing.B) { for i := 0; i < b.N; i++ { diff --git a/prover/protocol/compiler/lookup/conditional_test.go b/prover/protocol/compiler/lookup/conditional_test.go index d3b02a6dc..d20894309 100644 --- a/prover/protocol/compiler/lookup/conditional_test.go +++ b/prover/protocol/compiler/lookup/conditional_test.go @@ -119,7 +119,7 @@ func TestConditionalLogDerivativeLookupSimple2(t *testing.T) { // in the last appearance of 0 in filteredT expectedM := smartvectors.ForTest(1, 2, 0, 4) t.Logf("the list of columns is: %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_filterB_T_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_filterB,T_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") @@ -172,7 +172,7 @@ func TestConditionalLogDerivativeLookupManyChecksOneTable(t *testing.T) { // m should be expectedM := smartvectors.ForTest(0, 4, 4, 3) t.Logf("the list of columns is: %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_filterT_T_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_filterT,T_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") @@ -238,7 +238,7 @@ func TestConditionalLogDerivativeLookupOneXor(t *testing.T) { expectedM := smartvectors.ForTest(0, 1, 1, 1, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0) t.Logf("the list of columns is: %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_filterT_XOR_TABLE_X_XOR_TABLE_XXORY_XOR_TABLE_Y_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_filterT,XOR_TABLE_X,XOR_TABLE_XXORY,XOR_TABLE_Y_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") @@ -318,7 +318,7 @@ func TestConditionalLogDerivativeLookupMultiXor(t *testing.T) { // m should be expectedM := smartvectors.ForTest(1, 1, 1, 2, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 1, 0) t.Logf("the list of columns is: %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_FILTER_T_XOR_TABLE_X_XOR_TABLE_XXORY_XOR_TABLE_Y_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_FILTER_T,XOR_TABLE_X,XOR_TABLE_XXORY,XOR_TABLE_Y_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") @@ -432,7 +432,7 @@ func TestMixedConditionalLogDerivativeLookupMultiXor(t *testing.T) { // m should be expectedM := smartvectors.ForTest(2, 2, 1, 2, 2, 0, 1, 3, 0, 0, 0, 0, 2, 1, 1, 0) // 16 rows are included t.Logf("the list of columns is: %v", runtime.Columns.ListAllKeys()) - actualM := runtime.GetColumn("TABLE_FILTER_T_XOR_TABLE_X_XOR_TABLE_XXORY_XOR_TABLE_Y_0_LOGDERIVATIVE_M") + actualM := runtime.GetColumn("TABLE_FILTER_T,XOR_TABLE_X,XOR_TABLE_XXORY,XOR_TABLE_Y_0_LOGDERIVATIVE_M") assert.Equal(t, expectedM.Pretty(), actualM.Pretty(), "m does not match the expected value") diff --git a/prover/protocol/compiler/lookup/prover.go b/prover/protocol/compiler/lookup/prover.go index 6d7807c06..063fa5a6b 100644 --- a/prover/protocol/compiler/lookup/prover.go +++ b/prover/protocol/compiler/lookup/prover.go @@ -171,7 +171,7 @@ func (a mAssignmentTask) run(run *wizard.ProverRuntime) { // otherwise. tCollapsed = make([]sv.SmartVector, len(a.T)) - // tCollapsed contains either the assignment of the Ss if the table is a + // sCollapsed contains either the assignment of the Ss if the table is a // single column (e.g. isMultiColumn=false) or their collapsed version // otherwise. sCollapsed = make([]sv.SmartVector, len(a.S)) @@ -211,6 +211,7 @@ func (a mAssignmentTask) run(run *wizard.ProverRuntime) { } var ( + // m is associated with tCollapsed // m stores the assignment to the column M as we build it. m = make([][]field.Element, len(a.T)) diff --git a/prover/protocol/compiler/lookup/utils.go b/prover/protocol/compiler/lookup/utils.go index b569c04c1..a6d4eeb27 100644 --- a/prover/protocol/compiler/lookup/utils.go +++ b/prover/protocol/compiler/lookup/utils.go @@ -102,7 +102,7 @@ func nameTable(t []table) string { for col := range t[0] { colNames[col] = string(t[0][col].GetColID()) } - return fmt.Sprintf("TABLE_%v", strings.Join(colNames, "_")) + return fmt.Sprintf("TABLE_%v", strings.Join(colNames, ",")) } fragNames := make([]string, len(t)) diff --git a/prover/protocol/compiler/lookup/verifier.go b/prover/protocol/compiler/lookup/verifier.go index ceb0d31f3..b5179574b 100644 --- a/prover/protocol/compiler/lookup/verifier.go +++ b/prover/protocol/compiler/lookup/verifier.go @@ -17,9 +17,7 @@ import ( // // The current implementation is for packed Zs type finalEvaluationCheck struct { - // Name is a string repr of the verifier action. It is used to format errors - // so that we can easily know which verifier action is at fault during the - // verification is at fault. + // the name of a lookupTable in the pack, this can help for debugging. Name string // ZOpenings lists all the openings of all the zCtx ZOpenings []query.LocalOpening @@ -37,7 +35,7 @@ func (f *finalEvaluationCheck) Run(run *wizard.VerifierRuntime) error { } if zSum != field.Zero() { - return fmt.Errorf("log-derivate lookup, the final evaluation check failed") + return fmt.Errorf("log-derivate lookup, the final evaluation check failed for %v,", f.Name) } return nil diff --git a/prover/protocol/compiler/lookup/z_packing.go b/prover/protocol/compiler/lookup/z_packing.go index 1be03c51c..d6d12ffbe 100644 --- a/prover/protocol/compiler/lookup/z_packing.go +++ b/prover/protocol/compiler/lookup/z_packing.go @@ -36,6 +36,7 @@ type zCtx struct { Zs []ifaces.Column // ZOpenings are the opening queries to the end of each Z. ZOpenings []query.LocalOpening + Name string } // check permutation and see how/where compile is called (see how to constracut z there) @@ -80,32 +81,32 @@ func (z *zCtx) compile(comp *wizard.CompiledIOP) { z.ZDenominatorBoarded[i] = zDenominator.Board() z.Zs[i] = comp.InsertCommit( - z.Round+1, + z.Round, deriveName[ifaces.ColID]("Z", comp.SelfRecursionCount, z.Round, z.Size, i), z.Size, ) - // consistency check - comp.InsertGlobal( - z.Round+1, - deriveName[ifaces.QueryID]("Z_CONSISTENCY", comp.SelfRecursionCount, z.Round, z.Size, i), + // initial condition + comp.InsertLocal( + z.Round, + deriveName[ifaces.QueryID]("Z_CONSISTENCY_START", comp.SelfRecursionCount, z.Round, z.Size, i), sym.Sub( zNumerator, sym.Mul( - sym.Sub(z.Zs[i], column.Shift(z.Zs[i], -1)), + z.Zs[i], zDenominator, ), ), ) - // complete the consistency by adding the edge-case at position 0 - comp.InsertLocal( - z.Round+1, - deriveName[ifaces.QueryID]("Z_CONSISTENCY_START", comp.SelfRecursionCount, z.Round, z.Size, i), + // consistency check + comp.InsertGlobal( + z.Round, + deriveName[ifaces.QueryID]("Z_CONSISTENCY", comp.SelfRecursionCount, z.Round, z.Size, i), sym.Sub( zNumerator, sym.Mul( - z.Zs[i], + sym.Sub(z.Zs[i], column.Shift(z.Zs[i], -1)), zDenominator, ), ), @@ -113,7 +114,7 @@ func (z *zCtx) compile(comp *wizard.CompiledIOP) { // local opening of the final value of the Z polynomial z.ZOpenings[i] = comp.InsertLocalOpening( - z.Round+1, + z.Round, deriveName[ifaces.QueryID]("Z_FINAL", comp.SelfRecursionCount, z.Round, z.Size, i), column.Shift(z.Zs[i], -1), ) diff --git a/prover/protocol/compiler/selfrecursion/selfrecursion_test.go b/prover/protocol/compiler/selfrecursion/selfrecursion_test.go index 21fe71019..de57faab5 100644 --- a/prover/protocol/compiler/selfrecursion/selfrecursion_test.go +++ b/prover/protocol/compiler/selfrecursion/selfrecursion_test.go @@ -227,7 +227,7 @@ func TestSelfRecursionMultiLayered(t *testing.T) { define, vortex.Compile( 2, - vortex.ForceNumOpenedColumns(16), + vortex.ForceNumOpenedColumns(tc.NumOpenCol), vortex.WithSISParams(&tc.SisInstance), ), selfrecursion.SelfRecurse, @@ -235,7 +235,7 @@ func TestSelfRecursionMultiLayered(t *testing.T) { compiler.Arcane(1<<8, 1<<10, false), vortex.Compile( 2, - vortex.ForceNumOpenedColumns(16), + vortex.ForceNumOpenedColumns(tc.NumOpenCol), vortex.WithSISParams(&tc.SisInstance), ), selfrecursion.SelfRecurse, @@ -243,7 +243,7 @@ func TestSelfRecursionMultiLayered(t *testing.T) { compiler.Arcane(1<<11, 1<<13, false), vortex.Compile( 2, - vortex.ForceNumOpenedColumns(16), + vortex.ForceNumOpenedColumns(tc.NumOpenCol), vortex.WithSISParams(&tc.SisInstance), ), dummy.Compile, diff --git a/prover/protocol/compiler/specialqueries/range_test.go b/prover/protocol/compiler/specialqueries/range_test.go new file mode 100644 index 000000000..cc2d25190 --- /dev/null +++ b/prover/protocol/compiler/specialqueries/range_test.go @@ -0,0 +1,37 @@ +package specialqueries + +import ( + "testing" + + "github.com/consensys/linea-monorepo/prover/maths/common/smartvectors" + "github.com/consensys/linea-monorepo/prover/protocol/compiler/dummy" + "github.com/consensys/linea-monorepo/prover/protocol/compiler/lookup" + "github.com/consensys/linea-monorepo/prover/protocol/wizard" + "github.com/stretchr/testify/require" +) + +func TestRangWithLogDerivCompiler(t *testing.T) { + + define := func(b *wizard.Builder) { + cola := b.RegisterCommit("A", 8) + colb := b.RegisterCommit("B", 8) + + b.Range("RANG1", cola, 1<<4) + b.Range("RANG2", colb, 1<<4) + } + + prover := func(run *wizard.ProverRuntime) { + // assign a and b + cola := smartvectors.ForTest(15, 0, 12, 14, 9, 6, 2, 1) + + colb := smartvectors.ForTest(0, 6, 9, 1, 1, 5, 11, 1) + + run.AssignColumn("A", cola) + run.AssignColumn("B", colb) + } + + comp := wizard.Compile(define, RangeProof, lookup.CompileLogDerivative, dummy.Compile) + proof := wizard.Prove(comp, prover) + err := wizard.Verify(comp, proof) + require.NoError(t, err) +} diff --git a/prover/protocol/wizardutils/evaluation.go b/prover/protocol/wizardutils/evaluation.go index 7381060ac..d2a7096be 100644 --- a/prover/protocol/wizardutils/evaluation.go +++ b/prover/protocol/wizardutils/evaluation.go @@ -51,13 +51,22 @@ func EvalExprColumn(run *wizard.ProverRuntime, board symbolic.ExpressionBoard) s // returns the symbolic expression of a column obtained as a random linear combinations of differents handles // without committing to the column itself -func RandLinCombColSymbolic(x coin.Info, hs []ifaces.Column) *symbolic.Expression { +// @Azam this function is temporarily ignored and addressed in issue https://github.com/Consensys/linea-monorepo/issues/192 +/*func RandLinCombColSymbolic(x coin.Info, hs []ifaces.Column) *symbolic.Expression { cols := make([]*symbolic.Expression, len(hs)) for c := range cols { cols[c] = ifaces.ColumnAsVariable(hs[c]) } expr := symbolic.NewPolyEval(x.AsVariable(), cols) return expr +}*/ +func RandLinCombColSymbolic(x coin.Info, hs []ifaces.Column) *symbolic.Expression { + res := symbolic.NewConstant(0) + for i := len(hs) - 1; i >= 0; i-- { + res = symbolic.Mul(res, x) + res = symbolic.Add(res, hs[i]) + } + return res } // return the runtime assignments of a linear combination column From 8861b6dc00419c37e4df7fdcdcc028aa9cecd9da Mon Sep 17 00:00:00 2001 From: Azam Soleimanian <49027816+Soleimani193@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:11:22 +0200 Subject: [PATCH 05/12] fixing the round problem in the inner-product compiler (#201) --- .../compiler/innerproduct/compiler.go | 27 ++++- .../protocol/compiler/innerproduct/context.go | 41 ++++--- .../innerproduct/innerproduct_test.go | 107 ++++++++++++++++++ 3 files changed, 157 insertions(+), 18 deletions(-) create mode 100644 prover/protocol/compiler/innerproduct/innerproduct_test.go diff --git a/prover/protocol/compiler/innerproduct/compiler.go b/prover/protocol/compiler/innerproduct/compiler.go index 2e5291d52..bfe76159f 100644 --- a/prover/protocol/compiler/innerproduct/compiler.go +++ b/prover/protocol/compiler/innerproduct/compiler.go @@ -31,9 +31,11 @@ func Compile(comp *wizard.CompiledIOP) { // same protocol if the same step of compilation are applied in the same // order. sizes = []int{} - // contextsForSize list all the sub-compilation context in the same - // order as `sizes` - proverTask proverTask + // contextsForSize list all the sub-compilation context + // in the same order as `sizes`. + // proverTaskCollaps indicates when we have more than one pair of inner-product with the same size + // and thus collapsing all pairs to a single column is required. + proverTaskNoCollaps, proverTaskCollpas proverTask ) for _, qName := range comp.QueriesParams.AllUnignoredKeys() { @@ -60,9 +62,24 @@ func Compile(comp *wizard.CompiledIOP) { } for _, size := range sizes { - proverTask = append(proverTask, compileForSize(comp, round, queryMap[size])) + ctx := compileForSize(comp, round, queryMap[size]) + switch ctx.round { + case round: + proverTaskNoCollaps = append(proverTaskNoCollaps, ctx) + case round + 1: + proverTaskCollpas = append(proverTaskCollpas, ctx) + default: + utils.Panic("round before compilation was %v and after compilation %v", round, ctx.round) + } + + } + // run the prover of the relevant round + if len(proverTaskNoCollaps) >= 1 { + comp.RegisterProverAction(round, proverTaskNoCollaps) } - comp.RegisterProverAction(round+1, proverTask) + if len(proverTaskCollpas) >= 1 { + comp.RegisterProverAction(round+1, proverTaskCollpas) + } } diff --git a/prover/protocol/compiler/innerproduct/context.go b/prover/protocol/compiler/innerproduct/context.go index 789b1d82f..14eb78737 100644 --- a/prover/protocol/compiler/innerproduct/context.go +++ b/prover/protocol/compiler/innerproduct/context.go @@ -34,6 +34,9 @@ type contextForSize struct { // entry of [Summation]. It is compared to the alleged inner-product values // by the verifier to finalize the compilation step.s SummationOpening query.LocalOpening + + // round after compilation + round int } // compileForSize applies the compilation step on a range of queries such that @@ -41,6 +44,7 @@ type contextForSize struct { // list of queries. // // It returns the compilation context of the query +// the round indicate the round of the last inner-product query, independent of its size. func compileForSize( comp *wizard.CompiledIOP, round int, @@ -60,10 +64,12 @@ func compileForSize( if hasMoreThan1Pair { round = round + 1 } + //set the round + ctx.round = round ctx.Summation = comp.InsertCommit( - round+1, - deriveName[ifaces.ColID]("SUMMATION", comp.SelfRecursionCount), + round, + deriveName[ifaces.ColID]("SUMMATION", size, comp.SelfRecursionCount), size, ) @@ -74,8 +80,8 @@ func compileForSize( ) batchingCoin = comp.InsertCoin( - round+1, - deriveName[coin.Name]("BATCHING_COIN", comp.SelfRecursionCount), + round, + deriveName[coin.Name]("BATCHING_COIN", size, comp.SelfRecursionCount), coin.Field, ) @@ -85,8 +91,16 @@ func compileForSize( } } - ctx.Collapsed = symbolic.NewPolyEval(batchingCoin.AsVariable(), pairProduct) - ctx.Collapsed.Board() + // @Azam the following function is commented out due to the issue https://github.com/Consensys/linea-monorepo/issues/192 + // ctx.Collapsed = symbolic.NewPolyEval(batchingCoin.AsVariable(), pairProduct) + res := symbolic.NewConstant(0) + for i := len(pairProduct) - 1; i >= 0; i-- { + res = symbolic.Mul(res, batchingCoin) + res = symbolic.Add(res, pairProduct[i]) + } + + ctx.Collapsed = res + ctx.CollapsedBoard = ctx.Collapsed.Board() } if !hasMoreThan1Pair { @@ -96,8 +110,8 @@ func compileForSize( // This constraints set the recurrent property of summation comp.InsertGlobal( - round+1, - deriveName[ifaces.QueryID]("SUMMATION_CONSISTENCY", comp.SelfRecursionCount), + round, + deriveName[ifaces.QueryID]("SUMMATION_CONSISTENCY", size, comp.SelfRecursionCount), symbolic.Sub( ctx.Summation, column.Shift(ctx.Summation, -1), @@ -107,20 +121,21 @@ func compileForSize( // This constraint ensures that summation has the correct initial value comp.InsertLocal( - round+1, - deriveName[ifaces.QueryID]("SUMMATION_INIT", comp.SelfRecursionCount), + round, + deriveName[ifaces.QueryID]("SUMMATION_INIT", size, comp.SelfRecursionCount), symbolic.Sub(ctx.Collapsed, ctx.Summation), ) // The opening of the final position of ctx.Summation should be equal to // the linear combinations of the alleged openings of the inner-products. ctx.SummationOpening = comp.InsertLocalOpening( - round+1, - deriveName[ifaces.QueryID]("SUMMATION_END", comp.SelfRecursionCount), + round, + deriveName[ifaces.QueryID]("SUMMATION_END", size, comp.SelfRecursionCount), column.Shift(ctx.Summation, -1), ) - comp.RegisterVerifierAction(round+1, &verifierForSize{ + lastRound := comp.NumRounds() - 1 + comp.RegisterVerifierAction(lastRound, &verifierForSize{ Queries: queries, SummationOpening: ctx.SummationOpening, BatchOpening: batchingCoin, diff --git a/prover/protocol/compiler/innerproduct/innerproduct_test.go b/prover/protocol/compiler/innerproduct/innerproduct_test.go new file mode 100644 index 000000000..0a9afb899 --- /dev/null +++ b/prover/protocol/compiler/innerproduct/innerproduct_test.go @@ -0,0 +1,107 @@ +package innerproduct + +import ( + "testing" + + "github.com/consensys/linea-monorepo/prover/maths/common/smartvectors" + "github.com/consensys/linea-monorepo/prover/maths/field" + "github.com/consensys/linea-monorepo/prover/protocol/coin" + "github.com/consensys/linea-monorepo/prover/protocol/compiler/dummy" + "github.com/consensys/linea-monorepo/prover/protocol/ifaces" + "github.com/consensys/linea-monorepo/prover/protocol/wizard" + "github.com/stretchr/testify/assert" +) + +func TestInnerProduct(t *testing.T) { + define := func(b *wizard.Builder) { + for i, c := range testCases { + bs := make([]ifaces.Column, len(c.bName)) + a := b.RegisterCommit(c.aName, c.size) + for i, name := range c.bName { + bs[i] = b.RegisterCommit(name, c.size) + } + b.InnerProduct(c.qName, a, bs...) + // go to the next round + _ = b.RegisterRandomCoin(coin.Namef("Coin_%v", i), coin.Field) + } + } + prover := func(run *wizard.ProverRuntime) { + for j, c := range testCases { + run.AssignColumn(c.aName, c.a) + for i, name := range c.bName { + run.AssignColumn(name, c.b[i]) + } + run.AssignInnerProduct(c.qName, c.expected...) + run.GetRandomCoinField(coin.Namef("Coin_%v", j)) + } + } + + comp := wizard.Compile(define, Compile, dummy.Compile) + proof := wizard.Prove(comp, prover) + assert.NoErrorf(t, wizard.Verify(comp, proof), "invalid proof") +} + +var testCases = []struct { + qName ifaces.QueryID + aName ifaces.ColID + bName []ifaces.ColID + size int + a smartvectors.SmartVector + b []smartvectors.SmartVector + expected []field.Element +}{ + {qName: "Quey1", + aName: "ColA1", + bName: []ifaces.ColID{"ColB1"}, + size: 4, + a: smartvectors.ForTest(1, 1, 1, 1), + b: []smartvectors.SmartVector{ + smartvectors.ForTest(0, 3, 0, 2), + }, + expected: []field.Element{field.NewElement(5)}, + }, + {qName: "Quey2", + aName: "ColA2", + bName: []ifaces.ColID{"ColB2_0", "ColB2_1"}, + size: 4, + a: smartvectors.ForTest(1, 1, 1, 1), + b: []smartvectors.SmartVector{ + smartvectors.ForTest(0, 3, 0, 2), + smartvectors.ForTest(1, 0, 0, 2), + }, + expected: []field.Element{field.NewElement(5), field.NewElement(3)}, + }, + {qName: "Quey3", + aName: "ColA3", + bName: []ifaces.ColID{"ColB3_0", "ColB3_1"}, + size: 8, + a: smartvectors.ForTest(1, 1, 1, 1, 2, 0, 2, 0), + b: []smartvectors.SmartVector{ + smartvectors.ForTest(0, 3, 0, 2, 1, 0, 0, 0), + smartvectors.ForTest(1, 0, 0, 2, 1, 0, 0, 0), + }, + expected: []field.Element{field.NewElement(7), field.NewElement(5)}, + }, + {qName: "Quey4", + aName: "ColA4", + bName: []ifaces.ColID{"ColB4"}, + size: 16, + a: smartvectors.ForTest(1, 1, 1, 1, 2, 0, 2, 0, 1, 1, 1, 1, 1, 1, 1, 1), + b: []smartvectors.SmartVector{ + smartvectors.ForTest(0, 3, 0, 2, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1), + }, + expected: []field.Element{field.NewElement(15)}, + }, + + {qName: "Quey", + + aName: "ColA", + bName: []ifaces.ColID{"ColB"}, + size: 32, + a: smartvectors.ForTest(1, 1, 1, 1, 2, 0, 2, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 0, 2, 0, 1, 1, 1, 1, 1, 1, 1, 1), + b: []smartvectors.SmartVector{ + smartvectors.ForTest(0, 3, 0, 2, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 0, 3, 0, 2, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1), + }, + expected: []field.Element{field.NewElement(30)}, + }, +} 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 06/12] =?UTF-8?q?feat:=20rewrite=20error=20logs=20from=20b?= =?UTF-8?q?lob=20submission=20eth=5Fcall=20due=20to=20insuffi=E2=80=A6=20(?= =?UTF-8?q?#157)?= 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 07/12] 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 08/12] 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 09/12] 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 10/12] [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 11/12] [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; }); }); }); From 62438cb7e929ac0423c79073f711debbec31e5d3 Mon Sep 17 00:00:00 2001 From: Azam Soleimanian <49027816+Soleimani193@users.noreply.github.com> Date: Mon, 21 Oct 2024 11:42:35 +0200 Subject: [PATCH 12/12] added verifier check in sticker, fixed the edge case in product (#204) * added the verifier check for local opening * fixed the edge case for product --------- Co-authored-by: AlexandreBelling --- .../compiler/splitter/sticker/sticker.go | 18 ++++++++++++++++++ prover/symbolic/product.go | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/prover/protocol/compiler/splitter/sticker/sticker.go b/prover/protocol/compiler/splitter/sticker/sticker.go index 9e3d0ee53..84b87aacc 100644 --- a/prover/protocol/compiler/splitter/sticker/sticker.go +++ b/prover/protocol/compiler/splitter/sticker/sticker.go @@ -1,8 +1,10 @@ package sticker import ( + "fmt" "strings" + "github.com/consensys/gnark/frontend" "github.com/consensys/linea-monorepo/prover/maths/common/smartvectors" "github.com/consensys/linea-monorepo/prover/maths/common/vector" "github.com/consensys/linea-monorepo/prover/maths/field" @@ -434,6 +436,22 @@ func (ctx *stickContext) compileFixedEvaluation() { y := run.QueriesParams.MustGet(q.ID).(query.LocalOpeningParams).Y run.AssignLocalPoint(newQ.ID, y) }) + + // The verifier ensures that the old and new queries have the same assignement + ctx.comp.InsertVerifier(round, func(run *wizard.VerifierRuntime) error { + oldParams := run.GetLocalPointEvalParams(q.ID) + newParams := run.GetLocalPointEvalParams(queryName(q.ID)) + + if oldParams != newParams { + return fmt.Errorf("sticker verifier failed for local opening %v - %v", q.ID, queryName(q.ID)) + } + + return nil + }, func(api frontend.API, run *wizard.WizardVerifierCircuit) { + oldParams := run.GetLocalPointEvalParams(q.ID) + newParams := run.GetLocalPointEvalParams(queryName(q.ID)) + api.AssertIsEqual(oldParams.Y, newParams.Y) + }) } } diff --git a/prover/symbolic/product.go b/prover/symbolic/product.go index cd83b7f81..0e646dd1d 100644 --- a/prover/symbolic/product.go +++ b/prover/symbolic/product.go @@ -60,7 +60,7 @@ func NewProduct(items []*Expression, exponents []int) *Expression { } for i := range items { - if items[i].ESHash.IsZero() { + if items[i].ESHash.IsZero() && exponents[i] != 0 { return NewConstant(0) } }