-
Notifications
You must be signed in to change notification settings - Fork 127
Implement Valadium contracts for use in coordinator #1782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,165 @@ | |||
| package net.consensys.linea.contract.l1 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed from coordinator/clients/smart-contract-client/src/main/kotlin/net/consensys/linea/contract/l1/Web3JFunctionBuildersV6.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next time plase use a git mv to make a better diff :)
| } | ||
|
|
||
| override fun finalizedL2BlockTimestamp(blockParameter: BlockParameter): SafeFuture<ULong> { | ||
| TODO("Validium contract does not have currentTimestamp function") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| override fun finalizedL2BlockTimestamp(blockParameter: BlockParameter): SafeFuture<ULong> { | ||
| TODO("Validium contract does not have currentTimestamp function") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Validium client throws runtime error for interface method
The finalizedL2BlockTimestamp method uses TODO() which throws NotImplementedError at runtime. Since LineaValidiumSmartContractClientReadOnly extends LineaSmartContractClientReadOnly, callers expect this method to work. Any code that interacts with the Validium client through the common interface and calls this method will crash unexpectedly. If the Validium contract truly doesn't support this operation, a more appropriate exception type like UnsupportedOperationException with a descriptive message would provide better error handling.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1782 +/- ##
============================================
- Coverage 63.98% 63.31% -0.67%
- Complexity 1524 1528 +4
============================================
Files 404 407 +3
Lines 14545 14691 +146
Branches 1562 1565 +3
============================================
- Hits 9306 9301 -5
- Misses 4620 4771 +151
Partials 619 619
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| interface LineaRollupSmartContractClientReadOnly : ContractVersionProvider<LineaContractVersion> { | ||
| enum class LineaValidiumContractVersion : Comparable<LineaValidiumContractVersion> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this PR rebased on MAIN? these changes should already be on main branch, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR needs the Validium ABI which is in the main alt-da branch, so this change is on top of chore/design-alt-da-interfaces
The base branch was changed.
db5d2f1 to
845c930
Compare
845c930 to
2763531
Compare
| ) | ||
| } | ||
| .thenApply { it.transactionHash } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing synchronization on transaction sending method
The new sendShnarfDataTransactionAndGetTxHash method is missing the @Synchronized annotation that all other transaction-sending methods in this class have (sendTransaction, sendTransactionAsync, sendTransactionAfterEthCallAsync, sendBlobCarryingTransaction). This creates a potential race condition when multiple threads call acceptShnarfData concurrently, as the underlying createAndSendRawTransaction (which handles nonce management and signing) could be accessed without proper synchronization.
This PR implements issue(s) #
Checklist
Note
Adds Validium L1 contract support (clients/builders), refactors rollup builders into an object, extends Web3J helper, and updates interfaces/methods.
jvm-libs:linea:linea-contracts:l1-validiummodule (Web3J codegen forValidiumV1).Web3JLineaValidiumSmartContractClientReadOnlyandWeb3JLineaValidiumSmartContractClientwithacceptShnarfData*andfinalizeBlocks*flows.Web3JLineaValidiumFunctionBuildersforacceptShnarfDataandfinalizeBlocks.Web3JFunctionBuildersV6.ktwithWeb3JLineaRollupFunctionBuildersand update usages inWeb3JLineaRollupSmartContractClient.LineaValidiumContractVersionandLineaValidiumSmartContractClientReadOnly.acceptShnarfs*->acceptShnarfData*inLineaValidiumSmartContractClient.Web3JContractAsyncHelper):sendShnarfDataTransactionAndGetTxHashandexecuteEthCall(function, gasPriceCaps, ...)overload.settings.gradleand add dependency inlinea-contract-clients.Written by Cursor Bugbot for commit 2763531. This will update automatically on new commits. Configure here.