Create database table for blob data sidecar quarantine #7108
Conversation
etan-status
force-pushed
the
dev/etan/df-bq
branch
from
850a228 to
5572f3a
Compare
etan-status
changed the title
etan-status
changed the title
|
The quarantine DB works as is for blobs (and also for blocks in earlier commit), but it seems that a custom solution based on SQLite implicit rowid is preferred over explicit rowid through primary key. |
| template tableName(sidecar: typedesc[ForkyDataSidecar]): string = | ||
| when sidecar is deneb.BlobSidecar: | ||
| "electra_sidecars_quarantine" | ||
| else: |
There was a problem hiding this comment.
In all of these, it would be more future-proof to adopt the structure used elsewhere in the CL of:
when sidecar is deneb.BlobSidecar:
...
elif sidecar is fulu.DataColumnSidecar:
...
else:
static: doAssert false/static: raiseAssert/{.error.}/...because the next DAS approach after PeerDAS likely won't be based around fulu.DataColumnSidecar and it'd be good to be able to catch all such issues at compile-time
| ): KvResult[QuarantineDB] = | ||
| let | ||
| electraDataSidecar = | ||
| ? backend.initDataSidecarStore(tableName(deneb.BlobSidecar)) |
There was a problem hiding this comment.
Maybe comment explicitly in some places that this means there's no compatibility guarantees, forward or backward, around this schema.
There was a problem hiding this comment.
Or alternatively just be very explicit, indeed, about the lack of durability/persistence between instances, so any Fulu quarantine tables made now don't constrain the future.
| name: string | ||
| ): KvResult[DataSidecarStore] = | ||
| if name == "": | ||
| return ok(DataSidecarStore()) |
There was a problem hiding this comment.
how does this early-exit path actually function, i.e. when something tries to use it?
|
|
||
| # Without this export compilation fails with error | ||
| # vendor\nim-chronicles\chronicles.nim(352, 21) Error: undeclared identifier: 'activeChroniclesStream' | ||
| # It actually does not needed, because chronicles is not used in this file, |
There was a problem hiding this comment.
- It actually isn't needed; or
- It actually is not needed
| # Without this export compilation fails with error | ||
| # vendor\nim-chronicles\chronicles.nim(352, 21) Error: undeclared identifier: 'activeChroniclesStream' | ||
| # It actually does not needed, because chronicles is not used in this file, | ||
| # but because decodeSZSSZ() generic and uses chronicles generic expansion |
cheatfate
force-pushed
the
dev/etan/df-bq
branch
from
d581663 to
079d0fa
Compare
Add low-level interface to persist blocks by block root and sidecars by block root + index. Also support deleting junk that's been finalized and deletion of all data associated with a block root. Can pass `db.getQuarantineDB()` into the quarantine to expose the new DB tables while hiding APIs that deal with trusted data, and then use the new `beacon_chain_db_quarantine.nim` APIs to store / fetch / delete.
Fix stack usage compilation warnings using `closureScope`.
Add tests.
cheatfate
force-pushed
the
dev/etan/df-bq
branch
from
4cb3423 to
3d7c3af
Compare
Add low-level interface to persist blob data sidecars by block root + index. Also support deleting junk that's been finalized and deletion of all data associated with a block root.
Can pass
db.getQuarantineDB()into the quarantine to expose the new DB tables while hiding APIs that deal with trusted data, and then use the newbeacon_chain_db_quarantine.nimAPIs to store / fetch / delete.