search

logos-co / nomos-node

Nomos blockchain node
38 stars 13 forks source link

chore: cryptarchia unit tests update #657

Closed romanzac closed 1 week ago

romanzac commented 1 month ago

Batch of tests for Cryptarchia to improve coverage

romanzac commented 1 month ago

I have a question about ParentMissing(Id) error. It is not returned from:

    pub fn get(&self, id: &Id) -> Option<&Branch<Id>> {
        self.branches.get(id)
    }

Is this lose relationship what we want ? If not, I will reimplement to return Result.

romanzac commented 1 month ago

OrphanMissing(Id) error is not used yet, so I left it alone for now.

youngjoon-lee commented 1 month ago

I have a question about ParentMissing(Id) error. It is not returned from:

    pub fn get(&self, id: &Id) -> Option<&Branch<Id>> {
        self.branches.get(id)
    }

Is this lose relationship what we want ? If not, I will reimplement to return Result.

Thank you! I think we can change this function to return ParentMissing, but I'd like to wait for @zeegomo's review first.

zeegomo commented 1 week ago

I have a question about ParentMissing(Id) error. It is not returned from:

    pub fn get(&self, id: &Id) -> Option<&Branch<Id>> {
        self.branches.get(id)
    }

Is this lose relationship what we want ? If not, I will reimplement to return Result.

Branches is a collection of all the branches known to the node. The get method is used to request a branch using its last block as id, I think an Option is the natural return type for a search in a collection. Anyway, ParentMissing would not be the correct error, as we're missing the block in id, not its parent. How this method is used in downstream users and the errors returned there should not propagate here