feat: add cl0 transaction tracking #987
Conversation
...0/src/main/scala/io/constellationnetwork/currency/l0/http/routes/ValidationErrorRoutes.scala
Outdated
Show resolved
Hide resolved
...nnetwork/dag/l0/infrastructure/snapshot/GlobalSnapshotStateChannelEventsProcessorSuite.scala
Outdated
Show resolved
Hide resolved
| validationErrorsR <- Ref[F].of(Vector.empty[(Artifact, ValidationError, Instant)]) | ||
| } yield | ||
| new ValidationErrorStorage[F, Artifact, ValidationError] { | ||
| override def addValidationError(artifact: Artifact, errors: ValidationError): F[Unit] = |
There was a problem hiding this comment.
override modifier is wrongly used. it overrides nothing as this code implements the trait, not overrides it or any parent class.
...la/io/constellationnetwork/node/shared/infrastructure/consensus/ValidationErrorStorage.scala
Outdated
Show resolved
Hide resolved
...la/io/constellationnetwork/node/shared/infrastructure/consensus/ValidationErrorStorage.scala
Outdated
Show resolved
Hide resolved
...la/io/constellationnetwork/node/shared/infrastructure/consensus/ValidationErrorStorage.scala
Outdated
Show resolved
Hide resolved
.../node-shared/src/main/scala/io/constellationnetwork/node/shared/modules/SharedServices.scala
Outdated
Show resolved
Hide resolved
...o/constellationnetwork/node/shared/infrastructure/consensus/MockValidationErrorStorage.scala
Outdated
Show resolved
Hide resolved
| import weaver.SimpleIOSuite | ||
|
|
||
| object ValidationErrorStorageSuite extends SimpleIOSuite { | ||
| implicit val eq: Eq[IndexedSeq[(Int, String)]] = Eq.instance((a, b) => a == b) |
There was a problem hiding this comment.
how it differs from expect.same?
...la/io/constellationnetwork/node/shared/infrastructure/consensus/ValidationErrorStorage.scala
Outdated
Show resolved
Hide resolved
| override protected val prefixPath: InternalUrlPrefix = "/currency" | ||
|
|
||
| override protected val public: HttpRoutes[F] = HttpRoutes.of[F] { | ||
| case GET -> Root / "validation-errors" => |
There was a problem hiding this comment.
I am wondering if it shouldn't be a sub-resource of /snapshot/events/validation-errors but if it is currency-framework specific then maybe currency/snapshot-event-validation-errors or something more specific 🤔
WDYT?
| @@ -43,7 +45,8 @@ object HttpApi { | |||
| nodeVersion: TessellationVersion, | |||
| httpCfg: HttpConfig, | |||
| maybeDataApplication: Option[BaseDataApplicationL0Service[F]], | |||
| maybeMetagraphVersion: Option[MetagraphVersion] | |||
| maybeMetagraphVersion: Option[MetagraphVersion], | |||
| currencySnapshotValidationErrorStorage: ValidationErrorStorage[F, CurrencySnapshotEvent, Option[BlockRejectionReason]] | |||
There was a problem hiding this comment.
Technically it doesn't store snapshot validation errors. These are errors of snapshot events so inputs for creating the currency snapshot.
effects-ai
force-pushed
the
cl0_transaction_tracking
branch
from
46fb287 to
acfa1c7
Compare
| @@ -356,7 +356,7 @@ lazy val rosetta = (project in file("modules/rosetta")) | |||
| lazy val dagL1 = (project in file("modules/dag-l1")) | |||
| .enablePlugins(AshScriptPlugin) | |||
| .enablePlugins(JavaAppPackaging) | |||
| .dependsOn(kernel, shared % "compile->compile;test->test", nodeShared, testShared % Test) | |||
| .dependsOn(kernel, shared % "compile->compile;test->test", nodeShared % "compile->compile;test->test", testShared % Test) | |||
There was a problem hiding this comment.
do we still need it? we already have a testShared module so there should be no point to share test phase of nodeShared 🤔
|
|
||
| implicit val encoder: Encoder[ValidationErrorStorageEntry[CurrencySnapshotEvent, Option[BlockRejectionReason]]] = | ||
| new Encoder[ValidationErrorStorageEntry[CurrencySnapshotEvent, Option[BlockRejectionReason]]] { | ||
| override def apply(a: ValidationErrorStorageEntry[CurrencySnapshotEvent, Option[BlockRejectionReason]]): Json = Json.obj( |
There was a problem hiding this comment.
Encoder does not implement a default implementation of apply, so you don't override it, but just implement it. override modifier is wrongly used here
| case currency.BlockEvent(value) => value.asJson | ||
| case currency.DataApplicationBlockEvent(_) => Json.Null | ||
| case currency.CurrencyMessageEvent(_) => Json.Null |
There was a problem hiding this comment.
why other events are ignored?
There was a problem hiding this comment.
They are place holders, will be removed with actual error codes.
...ala/io/constellationnetwork/dag/l0/infrastructure/snapshot/GlobalSnapshotTraverseSuite.scala
Show resolved
Hide resolved
| trait ValidationErrorStorage[F[_], Artifact, ValidationError] { | ||
| def addValidationError(artifact: Artifact, errors: ValidationError): F[Unit] | ||
| def addValidationErrors(errors: NonEmptyList[(Artifact, ValidationError)]): F[Unit] | ||
| def allValidationErrors: F[Vector[ValidationErrorStorageEntry[Artifact, ValidationError]]] | ||
| } |
There was a problem hiding this comment.
instead of repeating the same name over and over, it may be:
trait ValidationErrorStorage ... {
def add(artifact: Artifact, error: ValidationError): ...
def add(artifact: Artifact, errors: NonEmptyList[ValidationError]): ...
def get: F[Vector[...]]
}| validationErrorsR <- Ref[F].of(Vector.empty[ValidationErrorStorageEntry[Artifact, ValidationError]]) | ||
| } yield | ||
| new ValidationErrorStorage[F, Artifact, ValidationError] { | ||
| override def addValidationError(artifact: Artifact, error: ValidationError): F[Unit] = |
There was a problem hiding this comment.
again, you don't override a default implementation but just providing the first implementation, so it overrides nothing
| new ValidationErrorStorage[F, Artifact, ValidationError] { | ||
| override def addValidationError(artifact: Artifact, error: ValidationError): F[Unit] = | ||
| for { | ||
| timestamp <- Clock[F].realTimeInstant |
There was a problem hiding this comment.
do you ever match errors for equality? how the timestamp is gonna by used? those timestamps are local timestamps and won't match any other timestamp in the network
| val (headArtifact, headError) = errors.head | ||
| errors.foldLeft(addValidationError(headArtifact, headError)) { | ||
| case (acc, (artifact, error)) => | ||
| acc >> addValidationError(artifact, error) | ||
| } |
There was a problem hiding this comment.
instead of extracting the head and violating DRY rule, you can put an empty unit in the fold as a zero value and fold over all elements then. you can also use traverse probably, that would be easier than reducing values like that, since F is a monad and you can chain computations
...la/io/constellationnetwork/node/shared/infrastructure/snapshot/CurrencySnapshotCreator.scala
Show resolved
Hide resolved
.../node-shared/src/main/scala/io/constellationnetwork/node/shared/modules/SharedServices.scala
Show resolved
Hide resolved
No description provided.