Skip to content

Conversation

@j-berman
Copy link
Collaborator

@j-berman j-berman commented Oct 3, 2025

When the request includes block_ids, the daemon uses find_blockchain_supplement to identify the highest block hash passed in block_ids that the daemon also knows about, and then serves subsequent blocks contiguous to that block.

When block_ids_exclusive is false (default current behavior), the daemon includes the highest block requested in the response, in addition to contiguous blocks after it.

When block_ids_exclusive is true (new param), the daemon serves blocks starting from the block 1 higher than the highest known block included in block_ids. Thus, the daemon skips the common block known to the client and daemon. Clients can make sure the daemon is serving expected contiguous blocks to its highest known block by checking the first block's prev_id included in the response, and making sure it is equivalent to the block the client already knows about that was included in block_ids. This avoids the daemon serving 1 extra block it does not need to serve to the client, since the client should already know about that block.


This optimization is used in the FCMP++ integration wallet2 refresh, implemented as part of this PR refactoring wallet2 refresh: seraphis-migration#81

@selsta selsta added this to the fcmp++ hf milestone Oct 4, 2025
@iamamyth
Copy link

iamamyth commented Oct 5, 2025

skip_common_block seems a rather long-winded synonym for exclusive.

@j-berman j-berman force-pushed the daemon-skip-common-block branch from 0ee2c16 to ba0cfe4 Compare October 6, 2025 20:18
@j-berman j-berman changed the title Daemon RPC: /getblocks.bin optional block_ids_skip_common_block req param Daemon RPC: /getblocks.bin optional block_ids_exclusive req param Oct 6, 2025
@j-berman
Copy link
Collaborator Author

j-berman commented Oct 6, 2025

skip_common_block seems a rather long-winded synonym for exclusive.

Updated to use _exclusive in the latest instead

db_rtxn_guard rtxn_guard(m_db);
top_hash = m_db->top_block_hash(&total_height);
++total_height;
CHECK_AND_ASSERT_MES(total_height >= start_height, false, "chain height expected to be higher than start block");
Copy link

Choose a reason for hiding this comment

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

Minor issue, but the message here doesn't match the assertion (ignores equality); suggest something like: Start height cannot exceed chain height.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually it should really be total_height > start_height, good spot

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed to check total_height > start_height

Copy link

@iamamyth iamamyth left a comment

Choose a reason for hiding this comment

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

Looks OK, apart from the (easily resolved) inaccuracy of the assertion message.

When the request includes block_ids, the daemon uses
find_blockchain_supplement to identify the highest block hash
passed in block_ids that the daemon also knows about, and then
serves subsequent blocks contiguous to that block.

When block_ids_skip_exclusive is false (default current
behavior), the daemon includes the highest block requested in the
response, in addition to contiguous blocks after it.

When block_ids_skip_exclusive is true (new param), the daemon
serves blocks starting from the block 1 higher than the highest
known block included in block_ids. Thus, the daemon skips the
common block known to the client and daemon. Clients can make sure
the daemon is serving expected contiguous blocks to its highest
known block by checking the first block's prev_id included in the
response, and making sure it is equivalent to the block the client
already knows about that was included in block_ids. This avoids
the daemon serving 1 extra block it does not need to serve to the
client, since the client should already know about that block.

bl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants