Skip to content

Conversation

agnxsh
Copy link
Contributor

@agnxsh agnxsh commented Sep 22, 2025

No description provided.

Copy link

github-actions bot commented Sep 22, 2025

Unit Test Results

       15 files  ±0    3 005 suites  ±0   1h 10m 6s ⏱️ - 13m 37s
11 390 tests ±0  10 828 ✔️ ±0  562 💤 ±0  0 ±0 
72 956 runs  ±0  72 164 ✔️ ±0  792 💤 ±0  0 ±0 

Results for commit f801418. ± Comparison against base commit 62ba6e9.

♻️ This comment has been updated with latest results.

current_id = node_id

while custody_groups.lenu64 < custody_group_count:
let safe_count = min(custody_group_count, cfg.NUMBER_OF_CUSTODY_GROUPS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/ethereum/consensus-specs/blob/v1.6.0-beta.0/specs/fulu/das-core.md#get_custody_groups defines it as an error to do this:

    assert custody_group_count <= NUMBER_OF_CUSTODY_GROUPS

    # Skip computation if all groups are custodied
    if custody_group_count == NUMBER_OF_CUSTODY_GROUPS:
        return [CustodyIndex(i) for i in range(NUMBER_OF_CUSTODY_GROUPS)]

but that would make things more complicated to return anything like an Opt or Result, so there needs to be some default here. But sure, it's fine to effectively set this error handling as clamping.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can need the peer for blocks?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can need the peer for blocks?

That's compatible with not thinking it has any custody at all.

@tersec tersec merged commit 4f069fc into unstable Sep 23, 2025
12 checks passed
@tersec tersec deleted the hm branch September 23, 2025 11:38
let columns = dataColumnsOpt.valueOr: DataColumnSidecars @[]
debug "Inserting columns into database (backfill)",
indices = columns.mapIt($it[].index)
indices = columns.mapIt($it[].index).len
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapIt will cause the allocation of an extra seq and some extra processing - if we're just getting the length, this is wasteful

siddarthkay pushed a commit that referenced this pull request Sep 24, 2025
* column serving issue and other fixes

* another change

* remove downscore

* fixes
siddarthkay pushed a commit that referenced this pull request Sep 24, 2025
* column serving issue and other fixes

* another change

* remove downscore

* fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants