R3SOL-368 Move ledger flow code from components into a new library #6353

williamvigorr3 · 2024-10-01T07:43:08Z

Put ledger flow code into a new library. Uncoupled the new libraries dependencies and transitive dependencies from the avro schema. Pulling out code where needed into new modules (if there isn't a sensible place).

corda-jenkins-ci02 · 2024-10-01T13:52:16Z

Jenkins build for PR 6353 build 62

Build Successful:
Jar artifact version produced by this PR: 5.3.0.0-alpha-1729778193720
Helm chart version produced by this PR: 5.3.0-alpha.1729778193720
Helm chart pushed to: oci://corda-os-docker-dev.software.r3.com/helm-charts/pr-6353/corda
Helm chart Polaris score: 82

blsemo · 2024-10-08T10:42:27Z

...ib-utxo-flow/src/main/java/net/corda/ledger/lib/utxo/flow/impl/persistence/package-info.java

Why do all those impl packages now need package-info files? Exporting an impl is usually a smell...

If I don't export these then I get the following error (so it looks like the exports are needed by the ledger flow tests):

> Task :components:ledger:ledger-utxo-flow:resolve FAILED Resolution failed. Capabilities satisfying the following requirements could not be found: [<<INITIAL>>] ⇒ osgi.identity: (osgi.identity=ledger-utxo-flow-tests) ⇒ [ledger-utxo-flow-tests version=5.3.0.0-SNAPSHOT] ⇒ osgi.wiring.package: (osgi.wiring.package=net.corda.ledger.lib.utxo.flow.impl.timewindow)

These were exported in their original location (except libs/ledger-lib-utxo-flow/src/main/java/net/corda/ledger/lib/utxo/flow/impl/transaction/factory/impl/package-info.java I'll try and work out why I needed to export that).

For example:

https://github.com/corda/corda-runtime-os/blob/release/os/5.3/components/ledger/ledger-utxo-flow/src/main/java/net/corda/ledger/utxo/flow/impl/persistence/package-info.java

iirc, when we exported an impl it was to get an integration test to work or something like that.

I can't get the tests to resolve without libs/ledger-lib-utxo-flow/src/main/java/net/corda/ledger/lib/utxo/flow/impl/transaction/factory/impl/package-info.java (as well).

lankydan · 2024-10-08T15:59:23Z

Nothing has jumped out to me as being wrong when i reviewed the diff, will take a closer look though.

wzur-r3

Gradle configuration changes LGTM

… library

blsemo

Looks ok to me, just the one question...

blsemo · 2024-10-15T16:06:00Z

.../kotlin/net/corda/ledger/common/flow/impl/transaction/TransactionSignatureServiceOsgiImpl.kt

Question - shouldn't the OsgiImpl be in a different module, so that we can pull in the non-OSGi version to the new project?

We discussed this offline but concluded the annotations are Okay as corda-dlt will ignore them and do not use OSGi for dependancy injection.

…ib' into WillV/R3SOL-368-Extract-Ledger-Lib # Conflicts: # libs/application/application-impl/build.gradle

lankydan · 2024-10-23T08:57:46Z

Remember to target the feature branch.

The base branch was changed.

…lV/R3SOL-368-Extract-Ledger-Lib

lankydan · 2024-10-23T18:07:12Z

...c/smokeTest/kotlin/net/corda/applications/workers/smoketest/services/CryptoRestSmokeTests.kt

@@ -39,6 +38,7 @@ import java.net.http.HttpResponse
 import java.time.Duration
 import java.time.Instant
 import java.util.UUID
+import net.corda.crypto.core.avro.toAvro


how'd this pass detekt with this import here 🤔

Maybe detekt isn't running in the smokeTest dir.

blsemo · 2024-10-24T08:47:33Z

...src/test/kotlin/net/corda/cpk/read/impl/services/persistence/CpkChunksFileManagerImplTest.kt

+        fun AvroSecureHash.toCorda(): SecureHash =
+            SecureHashImpl(this.algorithm, this.bytes.array())


Wouldn't it be possible to have the toAvro/toCorda functions defined somewhere and reuse them? They seem to be defined an awful lot of times...
Maybe not one central place, but even reducing the numbers would be good.

Some got missed during refactoring. I fixed this now.

blsemo · 2024-10-24T08:49:51Z

libs/crypto/cipher-suite-impl/src/main/kotlin/net/corda/cipher/suite/impl/DigestServiceImpl.kt

@@ -22,6 +22,7 @@ import org.osgi.service.component.annotations.ReferenceScope.PROTOTYPE_REQUIRED
 import org.osgi.service.component.annotations.ServiceScope
 import java.io.InputStream

+// I think the merkle tree code only has a dep on other crypto modules for this and secure hash impl


Should this comment stay here? That looks like a note for work in progress?

I've removed this.

blsemo · 2024-10-24T08:51:06Z

libs/crypto/crypto-core-avro/src/main/kotlin/net/corda/crypto/core/avro/SecureHashExtension.kt

+fun AvroSecureHash.toCorda(): SecureHash =
+    SecureHashImpl(this.algorithm, this.bytes.array())
+
+fun SecureHash.toAvro(): AvroSecureHash =
+    AvroSecureHash(this.algorithm, ByteBuffer.wrap(bytes))


So you've got a central definition here, why not use that everywhere?

I fixed this now.

blsemo · 2024-10-24T08:51:51Z

libs/crypto/crypto-impl-utils/build.gradle

+description 'Common crypto implementation'
+
+dependencies {
+//    compileOnly "org.osgi:org.osgi.service.component.annotations"


Remove commented out stuff, please!

This is fixed.

blsemo · 2024-10-24T08:52:35Z

libs/crypto/crypto-impl-utils/src/main/kotlin/net/corda/crypto/impl/utils/WireUtils.kt

@@ -110,7 +111,7 @@ fun SignatureSpec.toWire(serializer: AlgorithmParameterSpecEncodingService): Cry
        else -> CryptoSignatureSpec(signatureName, null, null)
    }.also {
        CordaMetrics.Metric.Crypto.SignatureSpecTimer.builder()
-            .withTag(CordaMetrics.Tag.OperationName, TO_WIRE_OPERATION_NAME)
+            .withTag(CordaMetrics.Tag.OperationName, net.corda.crypto.impl.utils.TO_WIRE_OPERATION_NAME)


Is the fully qualified name required here?

I fixed this. I think Intelij sometimes does this on refactor.

blsemo · 2024-10-24T08:57:35Z

libs/ledger/ledger-lib-common-flow/build.gradle

+
+    implementation project(':libs:ledger:ledger-common-data')
+    implementation project(':libs:crypto:crypto-core')
+    implementation project(':libs:serialization:serialization-amqp-api')


Do we depend on this in the new project? If yes, then AMQP would not be a comfortable dependency...

This is just amqp-api. So I think it's Okay.

./gradlew :libs:serialization:serialization-amqp-api:dependencies --configuration runTimeClassPath runtimeClasspath - Runtime classpath of null/main. +--- net.corda:corda-api:5.3.0.16-beta+ -> 5.3.0.16-beta-1729750087895 | +--- net.corda:corda-application:5.3.0.16-beta-1729750087895 (c) | +--- org.jetbrains.kotlin:kotlin-osgi-bundle:1.8.21 (c) | +--- javax.persistence:javax.persistence-api:2.2 (c) | +--- net.corda:corda-base:5.3.0.16-beta-1729750087895 (c) | +--- net.corda:corda-crypto:5.3.0.16-beta-1729750087895 (c) | +--- net.corda:corda-membership:5.3.0.16-beta-1729750087895 (c) | +--- net.corda:corda-serialization:5.3.0.16-beta-1729750087895 (c) | \--- org.slf4j:slf4j-api:2.0.13 (c) +--- net.corda:corda-application -> 5.3.0.16-beta-1729750087895 | +--- javax.persistence:javax.persistence-api:2.2 | +--- net.corda:corda-api:5.3.0.16-beta-1729750087895 (*) | +--- net.corda:corda-base:5.3.0.16-beta-1729750087895 | | +--- net.corda:corda-api:5.3.0.16-beta-1729750087895 (*) | | \--- org.slf4j:slf4j-api:2.0.13 | +--- net.corda:corda-crypto:5.3.0.16-beta-1729750087895 | | +--- org.slf4j:slf4j-api:2.0.13 | | +--- net.corda:corda-api:5.3.0.16-beta-1729750087895 (*) | | \--- net.corda:corda-base:5.3.0.16-beta-1729750087895 (*) | +--- net.corda:corda-membership:5.3.0.16-beta-1729750087895 | | +--- net.corda:corda-api:5.3.0.16-beta-1729750087895 (*) | | \--- net.corda:corda-base:5.3.0.16-beta-1729750087895 (*) | \--- net.corda:corda-serialization:5.3.0.16-beta-1729750087895 | +--- net.corda:corda-api:5.3.0.16-beta-1729750087895 (*) | \--- net.corda:corda-base:5.3.0.16-beta-1729750087895 (*) \--- org.jetbrains.kotlin:kotlin-osgi-bundle:1.8.21

Only serialization-amqp depends on proton-j.

…ib' into WillV/R3SOL-368-Extract-Ledger-Lib

sonarqubecloud · 2024-10-24T13:05:29Z

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Move code from components into new library

c36fb62

williamvigorr3 force-pushed the WillV/R3SOL-368-Extract-Ledger-Lib branch from ac9b06e to c36fb62 Compare October 1, 2024 08:14

williamvigorr3 added 2 commits October 1, 2024 10:24

Remove assert on fully qualified class name.

03a696b

Move SingletonSerializeAsToken into the OSGi class

3adfc0e

williamvigorr3 added 4 commits October 1, 2024 15:59

Remove the dependency on sandbox from ledger-lib-utxo-flow

0f6c980

Remove the group parameters from ledger-lib-utxo-flow

81760d9

Tidy build.gradle

005b801

Tidy code

54c69ae

blsemo reviewed Oct 8, 2024

View reviewed changes

williamvigorr3 added 4 commits October 9, 2024 11:08

Split out NotarySignatureVerificationService

5d8f154

Pull flow service out of TransactionSignatureService

d9f4465

Use alpha corda-api version

ccaf798

Port unit tests into ledger-lib-utxo-flow

56dfc61

williamvigorr3 marked this pull request as ready for review October 14, 2024 12:32

williamvigorr3 requested review from a team as code owners October 14, 2024 12:32

wzur-r3 previously approved these changes Oct 15, 2024

View reviewed changes

Move ledger-common-flow-api and ledger-common-flow from components to…

3a8bd66

… library

williamvigorr3 dismissed wzur-r3’s stale review via 3a8bd66 October 15, 2024 14:46

williamvigorr3 requested a review from a team as a code owner October 15, 2024 14:46

Revert API version

34620ae

blsemo reviewed Oct 15, 2024

View reviewed changes

williamvigorr3 added 4 commits October 16, 2024 10:16

Remove virtual node dependency

73407a9

Fix unit tests

96492c3

Remove runtime dependency

b7d16a7

Remove avro dependency from flow-api

a746120

williamvigorr3 requested a review from a team as a code owner October 17, 2024 10:23

williamvigorr3 and others added 6 commits October 22, 2024 13:57

Delete accidental OSGi Test

eae1b81

Reduce dependencies in application

7b020cb

removed avro from crypto-impl and layered-property-map

2a062c0

Merge remote-tracking branch 'origin/WillV/R3SOL-368-Extract-Ledger-L…

39233ff

…ib' into WillV/R3SOL-368-Extract-Ledger-Lib # Conflicts: # libs/application/application-impl/build.gradle

removed avro from crypto-impl and layered-property-map

aa731b5

detekt

4f99ede

williamvigorr3 changed the base branch from release/os/5.3 to feature/ledger-lib October 23, 2024 08:52

lankydan previously approved these changes Oct 23, 2024

View reviewed changes

williamvigorr3 changed the base branch from feature/ledger-lib to feature/ledger-library October 23, 2024 12:02

williamvigorr3 added 2 commits October 23, 2024 13:14

Merge remote-tracking branch 'origin/feature/ledger-library' into Wil…

43e8c93

…lV/R3SOL-368-Extract-Ledger-Lib

Merge remote-tracking branch 'origin/feature/ledger-library' into Wil…

993073a

…lV/R3SOL-368-Extract-Ledger-Lib

lankydan reviewed Oct 23, 2024

View reviewed changes

lankydan added 2 commits October 23, 2024 19:22

import ordering

4096ddf

import ordering

0f8f364

lankydan previously approved these changes Oct 24, 2024

View reviewed changes

blsemo reviewed Oct 24, 2024

View reviewed changes

williamvigorr3 added 2 commits October 24, 2024 13:31

Respond to review comments

6220dab

Merge remote-tracking branch 'origin/WillV/R3SOL-368-Extract-Ledger-L…

2872d8f

…ib' into WillV/R3SOL-368-Extract-Ledger-Lib

williamvigorr3 dismissed lankydan’s stale review via 2872d8f October 24, 2024 12:34

williamvigorr3 added 2 commits October 24, 2024 13:43

Fix missed files

f353bc3

Remove comented out code.

0c9ce08

blsemo previously approved these changes Oct 24, 2024

View reviewed changes

Fix borken dependency

5186aad

williamvigorr3 dismissed blsemo’s stale review via 5186aad October 24, 2024 13:02

lankydan approved these changes Oct 24, 2024

View reviewed changes

williamvigorr3 merged commit c80cfb8 into feature/ledger-library Oct 24, 2024
5 checks passed

williamvigorr3 deleted the WillV/R3SOL-368-Extract-Ledger-Lib branch October 24, 2024 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R3SOL-368 Move ledger flow code from components into a new library #6353

R3SOL-368 Move ledger flow code from components into a new library #6353

williamvigorr3 commented Oct 1, 2024 •

edited

Loading

corda-jenkins-ci02 bot commented Oct 1, 2024 •

edited

Loading

blsemo Oct 8, 2024

williamvigorr3 Oct 8, 2024 •

edited

Loading

lankydan Oct 8, 2024

williamvigorr3 Oct 9, 2024

lankydan commented Oct 8, 2024

wzur-r3 left a comment

blsemo left a comment

blsemo Oct 15, 2024

williamvigorr3 Oct 16, 2024

lankydan commented Oct 23, 2024

lankydan Oct 23, 2024

williamvigorr3 Oct 23, 2024

blsemo Oct 24, 2024

williamvigorr3 Oct 24, 2024

blsemo Oct 24, 2024

williamvigorr3 Oct 24, 2024

blsemo Oct 24, 2024

williamvigorr3 Oct 24, 2024

blsemo Oct 24, 2024

williamvigorr3 Oct 24, 2024

blsemo Oct 24, 2024

williamvigorr3 Oct 24, 2024

blsemo Oct 24, 2024

williamvigorr3 Oct 24, 2024

sonarqubecloud bot commented Oct 24, 2024

		fun AvroSecureHash.toCorda(): SecureHash =
		SecureHashImpl(this.algorithm, this.bytes.array())

R3SOL-368 Move ledger flow code from components into a new library #6353

R3SOL-368 Move ledger flow code from components into a new library #6353

Conversation

williamvigorr3 commented Oct 1, 2024 • edited Loading

corda-jenkins-ci02 bot commented Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

williamvigorr3 Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lankydan commented Oct 8, 2024

wzur-r3 left a comment

Choose a reason for hiding this comment

blsemo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lankydan commented Oct 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Oct 24, 2024

Quality Gate passed

williamvigorr3 commented Oct 1, 2024 •

edited

Loading

corda-jenkins-ci02 bot commented Oct 1, 2024 •

edited

Loading

williamvigorr3 Oct 8, 2024 •

edited

Loading