-
Notifications
You must be signed in to change notification settings - Fork 12
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
DA API indexer implementation #644
Conversation
pub trait Next { | ||
fn next(self) -> Self; | ||
} | ||
|
||
pub trait Metadata { | ||
type AppId; | ||
type Index; | ||
type Index: Next; |
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.
I believe there is an Inc
trait somewhere 🤔 . Not important though as this works well.
impl<const SIZE: usize, C: VID> BlobCertificateSelect for FillSize<SIZE, C> { | ||
type Certificate = C; |
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.
Maybe we can use Vid instead of Certificate in the Select trait? (But is a refactor that can be done in a separated PR to keep this one simple).
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.
I was thinking to propose the VidCertificate
or VIDC
.
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.
Sounds good to me :)
nomos-da/full-replication/src/lib.rs
Outdated
@@ -16,6 +18,9 @@ use blake2::{ | |||
use bytes::Bytes; | |||
use serde::{Deserialize, Serialize}; | |||
|
|||
#[derive(Copy, Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] | |||
pub struct FRIndex([u8; 8]); |
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.
What is the FR
meaning? Imo is better to use self-explanatory naming.
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.
Will remove FR prefix (FullReplication), Index
should work, as it's coming from the full replication module.
// when full replication certificate is converted into the VID (which should happen after | ||
// the verification in the mempool) the id is set to the blob hash to allow identification | ||
// of the distributed data accross nomos nodes. | ||
let id = cert.attestations[0].blob_hash(); |
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.
Reading this in where we use indexing (cert.attestations[0]
) because we are supposed to know that this is called when Certificate is already validated. Made me realise that instead of having a From<Certificate>
maybe we need a TryFrom<Certificate>
instead. This is a basic example of our conversation the other day in where it may fail(?).
Another option would be to encode it in the type system something like:
pub struct Certificate<Verified>{
...
}
In where we transition to a verified state and we implement the a vid(&self) -> VID
method just for the verified state.
Im not sure how would that fit with the traits, but I think it should be possible. wdyt @bacv ?
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.
Certificate<Verified>
is a way to go, will create a new issue to track it.
let stream = storage_adapter.get_range_stream(app_id, range).await; | ||
let results = stream.collect::<Vec<_>>().await; | ||
|
||
let _ = reply_channel.send(results); |
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.
Lets debug log if it fails to send the result
// TODO: Using while loop here until `Step` trait is stable. | ||
// | ||
// For index_range to be used as Range with the stepping capabilities (eg. `for idx in | ||
// item_range`), Metadata::Index needs to implement `Step` trait, which is unstable. | ||
// See issue #42168 <https://github.com/rust-lang/rust/issues/42168> for more information. |
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.
Nice annotation. Was not aware of this!
// TODO: Rocksdb has a feature called BlobDB that handles largo blob storing, but further | ||
// investigation needs to be done to see if rust wrapper supports it. |
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.
Lets open an issue pointing at this. It is a nice point to check. Thanks!
|
||
// TODO: Rocksdb has a feature called BlobDB that handles largo blob storing, but further | ||
// investigation needs to be done to see if rust wrapper supports it. | ||
async fn load_blob(app_id: &[u8], id: &[u8]) -> Bytes { |
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 could probably just return an Option<Bytes>
or a Result<Bytes, SomeErr>
, as where is used we are expecting an Option
as well. The the lack of having the file or whatever is handled by the one that use it.
Like it is now we can end up with a Some(EmptyBytes)
which makes no sense. Better a None
or just EmptyBytes
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.
load_blob
returns Some(Bytes) only if read succeeds, any other case returns None now.
1623560
to
43fb0b9
Compare
} | ||
|
||
fn meta_to_key_bytes(app_id: impl AsRef<[u8]>, idx: impl AsRef<[u8]>) -> Bytes { | ||
let mut buffer = BytesMut::with_capacity(42 + app_id.as_ref().len() + idx.as_ref().len()); |
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.
42
better as a constant outside.
} | ||
|
||
fn meta_to_key_bytes(app_id: impl AsRef<[u8]>, idx: impl AsRef<[u8]>) -> Bytes { | ||
let mut buffer = BytesMut::with_capacity(42 + app_id.as_ref().len() + idx.as_ref().len()); |
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.
42
better as a constant.
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.
meta_to_key_bytes
was changed to key_bytes
, constant removed:
https://github.com/logos-co/nomos-node/pull/644/files#diff-bcd2717fdf8fd2ef24063fd29eae1b9bd1f96f1d75fb41982217f136d211a254R151-R158
You've probably reviewed commit by commit, sorry for not keeping the commits domain specific.
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.
@bacv I think I was having some issues and maybe I review an older version or who knows. Looks like everything is working well now.
* Rocksb adapter in da indexer * Handle da service messages * Remove indexer trait, use storage directly in the indexer service * Return unordered indexes range * Load blob by vid from file * Use VID in consensus * Change full replication index type to byte array * Change payload to cert and item to vid where required * Service integration tests for indexer * Feature gate rocksdb backend * Propagate range response send error * FRIndex to Index * VID to VidCertificate * Pass blobs file dir via settings
* Base cleaning of da to new traits/structure Added new da protocols and types * DA: KZG+RS core (#632) * Removed old kzg rs modules * Added new kzgrs core module * Implemented bytes_to_polynomial and tests * Use coefficient form * Refactor evaluations into method * Use domain elements instead of roots of unity in tests * Fix encoding and test * Clippy happy * Add comments * Implement polynomial commitment * Implement proof generation * Sketch fn signature for verification * implement proof verification * Implemented verification and tests * Return evaluations from bytes_to_polynomial as well * Use modular le bytes * Implement rs encode/decode * Implement decoding tests * Implement decode using lagrange * Cleanup imports * Da: v1 encoder (#633) * Added new kzgrs core module * Implemented bytes_to_polynomial and tests * Use coefficient form * Refactor evaluations into method * Use domain elements instead of roots of unity in tests * Fix encoding and test * Clippy happy * Add comments * Implement polynomial commitment * Implement proof generation * Sketch fn signature for verification * implement proof verification * Implemented verification and tests * Return evaluations from bytes_to_polynomial as well * Use modular le bytes * Implement rs encode/decode * Implement decoding tests * Implement decode using lagrange * Added chunksmatrix * Implement encoder with chunkify * Added missing files * Implement commit row commitments * Implement compute elements (row) proofs * Fix lib and types exposures * Implement encoder main methods * Implement encode method * Implement chunkify test Fix related bugs * Implement compute row kzg commitments Fix related bugs * Implement rs encode rows test Fix related bugs Refactored API * Implement row proofs tests Fix fieldelement encoding/decoding bug * Implement aggregated commitment test Implement aggregated column proofs test * Cleanup * Fix deps * Fix tests * Fix chunk too big test * Da: v1 verifier (#635) * Fix encoding and test * Implement commit row commitments * Implemented dablob * Implement verifier new Implement verifier check column * Clippy cleanup * Implement verifier * Implemented verify column test * Implemented full verify test * DA API Payload to Item in mempool (#634) * Base cleaning of da to new traits/structure Added new da protocols and types * DA: KZG+RS core (#632) * Removed old kzg rs modules * Added new kzgrs core module * Implemented bytes_to_polynomial and tests * Use coefficient form * Refactor evaluations into method * Use domain elements instead of roots of unity in tests * Fix encoding and test * Clippy happy * Add comments * Implement polynomial commitment * Implement proof generation * Sketch fn signature for verification * implement proof verification * Implemented verification and tests * Return evaluations from bytes_to_polynomial as well * Use modular le bytes * Implement rs encode/decode * Implement decoding tests * Implement decode using lagrange * Cleanup imports * Reduce abstraction for certificate and vid metadata * Allow payload to mempool as long as it converts into item * Da Certificate verifier * Add mock certificate for core tests * Mempool payload verification * Integrate mock verifiers for tx and certs * Detach verification from cert and tx * Seperate payload and item in mempools * Mempools in integration tests * Remove old cert verifier * Network payload to item constraints in da mempool * Update nomos-da/full-replication/src/lib.rs Co-authored-by: Daniel Sanchez <sanchez.quiros.daniel@gmail.com> * Sort attestations for cert signature * Update nomos-da/full-replication/src/lib.rs Co-authored-by: Daniel Sanchez <sanchez.quiros.daniel@gmail.com> --------- Co-authored-by: danielsanchezq <sanchez.quiros.daniel@gmail.com> * DA API Certificate verification (#641) * Redo certificate verification in mempool * FullReplication verifier params provider * Integrate da params provider into the node * DA API Indexer service (#643) * Base cleaning of da to new traits/structure Added new da protocols and types * Remove da availability crate completely * Scaffold for da storage service * Indexer service responsible for storage and blockchain subscription * Handle index related ops only * Acquire storage and consensus relays * Indexer trait * wip: storage adapter * Use storage adapter instead of storage * Add consensus adapter trait for block subscriptions * Consensus block subscriber adapter * Use metadata from da core in indexer * Update nomos-services/data-availability/indexer/src/lib.rs Co-authored-by: Daniel Sanchez <sanchez.quiros.daniel@gmail.com> * Update nomos-services/data-availability/indexer/src/lib.rs Co-authored-by: Daniel Sanchez <sanchez.quiros.daniel@gmail.com> * Use std::ops::Range for da queries * Return certificate metadata * Da storage adapter methods --------- Co-authored-by: danielsanchezq <sanchez.quiros.daniel@gmail.com> * Reuse evaluations when computing proofs (#647) * Reuse precomputed evaluations instead of evaluation polynomial for each proof * kzgrs benchmarks * Clippy happy * DA API indexer implementation (#644) * Rocksb adapter in da indexer * Handle da service messages * Remove indexer trait, use storage directly in the indexer service * Return unordered indexes range * Load blob by vid from file * Use VID in consensus * Change full replication index type to byte array * Change payload to cert and item to vid where required * Service integration tests for indexer * Feature gate rocksdb backend * Propagate range response send error * FRIndex to Index * VID to VidCertificate * Pass blobs file dir via settings * Da v1 multiple proofs bench (#648) * Parallel proof generation bench * Added bench comment * Modify domain to fit exact sizes * Fix domain in benches * Force parallelization features in lib * DA: Implement base structure for verifier service (#627) * Base cleaning of da to new traits/structure Added new da protocols and types * Implement base structure for verifier service * Added comments and todo! * Cleanup imports * Size of VidCert in full replication * Nomos Da Verifier service crate * Extension replaced with metadata * Fix DaIndexer service name * Storage adapter trait in verifier * Manage lifecycle and messages in verifier * Blob trait in core * Common nomos da storage crate * Use updated nomos da storage in indexer * Verifier storage adapter * Libp2p adaper for verifier * Kzgrs backend in verifier service * Fix fmt * Clippy happy --------- Co-authored-by: Gusto <bacvinka@gmail.com> * DA Verifier service integration tests (#650) * Base cleaning of da to new traits/structure Added new da protocols and types * Implement base structure for verifier service * Added comments and todo! * Cleanup imports * Size of VidCert in full replication * Nomos Da Verifier service crate * Extension replaced with metadata * Storage adapter trait in verifier * Manage lifecycle and messages in verifier * Common nomos da storage crate * Use updated nomos da storage in indexer * Verifier storage adapter * Libp2p adaper for verifier * Kzgrs backend in verifier service * Fix fmt * Data availability tests module * Return attestation in service msg response * Common definitions for da tests * Serde for kzgrs proofs and commitments * Da verifier integration test * WIP nomos-core in kzgrs backend * Kzgrs blob to common module * Add client zone to verifier test and check if attestations are created * Cleanup and comments * Use libp2p only for verifier and indexer service tests * Lint in da tests * Simplify da blob serialization * Remove attester from nomos-core attestation * Verifier backend error --------- Co-authored-by: danielsanchezq <sanchez.quiros.daniel@gmail.com> * DA Kzgrs Backend Certificate implementation (#651) * Kzgrs backend certificate definition * Encoded data to certificate test * Nomos da domain specific tag * Handle errors in da certificate creation * Add nomos core traits to da cert * Derive ordering traits for da index * Add failure test cases to kzgrs certificate * Da v1 benches expand (#658) * Update benches with more cases * Expand benches * Added parallel feature * Fix test comment Co-authored-by: Youngjoon Lee <5462944+youngjoon-lee@users.noreply.github.com> * Remove outdated comment --------- Co-authored-by: Gusto <bacvinka@gmail.com> Co-authored-by: gusto <bacv@users.noreply.github.com> Co-authored-by: Youngjoon Lee <5462944+youngjoon-lee@users.noreply.github.com>
Indexer implementation and service integration tests.
Recap of the certificate to the index steps:
app+index
is used as a key in the rocksdb backend and thevid
set as value.When caller asks for range of indexes for particular
app_id
, indexer just queries for that range in the rocksdb-adapter, in response replacingapp+index=vid
toapp+index=Option<Blob>
.The blob storage for now is assumed to be filesystem -
app_id
as a directory andvid
as a filename for the blob bytes. Theconst
directory should be replaced from the one provided in a settings, this could be done when verifier and indexer are integrated together.Rocksdb could be used for blob storage, it has a feature called BlobDB(this might be a separate implementation/version), but the usage of it is not clear at the moment.
To run the service integration test: