[node] AttestBatch endpoint
#676
Conversation
ian-shim
force-pushed
the
node-attest-batch
branch
3 times, most recently
from
bb1a565 to
d5513cf
Compare
| // GetBatchHeader constructs a core.BatchHeader from a proto of pb.StoreChunksRequest. | ||
| // Note the StoreChunksRequest is validated as soon as it enters the node gRPC | ||
| // interface, see grpc.Server.validateStoreChunkRequest. | ||
| func GetBatchHeader(in *pb.BatchHeader) (*core.BatchHeader, error) { |
There was a problem hiding this comment.
moving a bunch of util methods from node/grpc/utils.go to prevent circular dependency
ian-shim
force-pushed
the
node-attest-batch
branch
from
d5513cf to
20665f3
Compare
| ) | ||
|
|
||
| const ( |
There was a problem hiding this comment.
store related utils are moved to store_utils.go
node/store.go
Outdated
| @@ -223,6 +223,13 @@ func (s *Store) deleteNBatches(currentTimeUnixSec int64, numBatches int) (int, e | |||
| size += int64(len(blobIter.Value())) | |||
| } | |||
| blobIter.Release() | |||
|
|
|||
| // Blob index. | |||
| blobIndexIter := s.db.NewIterator(EncodeBlobIndexKeyPrefix(batchHeaderHash)) | |||
There was a problem hiding this comment.
If the intent is to mostly deprecate this function, should we put this code somewhere else?
There was a problem hiding this comment.
Which function are you referring to? deleteNBatches or EncodeBlobIndexKeyPrefix?
There was a problem hiding this comment.
Referring to deleteNBatches. It's currently handling both "Batches" which will be deprecated and "Batch Mappings" which won't be?
There was a problem hiding this comment.
Sure, agree having a separate method would make it easier to deprecate the functionality in the future.
node/store.go
Outdated
| values := make([][]byte, 0) | ||
|
|
||
| expirationTime := s.expirationTime() | ||
| expirationKey := EncodeBatchExpirationKey(expirationTime) |
There was a problem hiding this comment.
We allow multiple blobs to have the same expiration time, but not for batches. Is this correct? Esp. if we were to allow multiple dispersers?
There was a problem hiding this comment.
Yes, a blob has its own expiration time. This would just expire the batch <-> blob mapping.
There was a problem hiding this comment.
What if there happen to be two batches that get created with the same expiration time? It seems like we're worrying about that case for blobs but not for batch mappings.
There was a problem hiding this comment.
I see what you mean. That's a valid edge case.
In fact, the existing StoreChunks endpoint would have this issue as well.
I also see an issue with how we're adding expirations for blobs - there could be a concurrency issue that can void a write without a lock.
I'll refactor the expiration key such that the expiration key is in format:
_EXPIRATION_<EXPIRATION_TIME><BATCH_HEADER_HASH>-> batch header hash_BLOBEXPIRATION_<EXPIRATION_TIME><BLOB_HEADER_HASH>-> blob header hash
This way we can still query expirations by expiration time, but each batch/blob gets its own record. Wdyt?
cc @jianoaix
ian-shim
force-pushed
the
node-attest-batch
branch
from
20665f3 to
219fc21
Compare
ian-shim
force-pushed
the
node-attest-batch
branch
from
2880ff6 to
a92767d
Compare
ian-shim
force-pushed
the
node-attest-batch
branch
from
a92767d to
1063fa9
Compare
Why are these changes needed?
This PR adds
AttestBatchendpoint to DA node.This endpoint can be used to attest to a batch of blobs represented by a list of blob header hashes that the node had previously received.
Checks