Skip to content

Txs batching change merge #1

Open
knoxrm wants to merge 2 commits into
mainfrom
txs-batching-change
Open

Txs batching change merge #1
knoxrm wants to merge 2 commits into
mainfrom
txs-batching-change

Conversation

@knoxrm

@knoxrm knoxrm commented Dec 17, 2024

Copy link
Copy Markdown

This branch was made to move all the batching to use byte size of the transaction/wallet/requests instead of the size of the queue of the transactions/wallet/requests

Transaction processing is now based bytes size of the transactions
Comment thread src/core/cbdc_udl.cpp
wait_start = now;

if(!wallet_queue.empty() &&
(curr_batch_size < udl->config.wallet_persistence_batch_min_size) ||

@etremel etremel Dec 11, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems like a logic error because if curr_batch_size and batch_min_size are both 0, the expression will never be true. Since the body of the if statement adds another wallet to the batch, it seems like the head of the if statement should either check if curr_batch_size is greater than the minimum, or less than the maximum (e.g. batch_max_size).

Comment thread src/core/cbdc_udl.cpp
(curr_batch_size < udl->config.wallet_persistence_batch_min_size) ||
((now-wait_start) >= batch_time)) {
std::size_t wallet_size = mutils::bytes_size(std::get<1>(wallet_queue.front()));
if (curr_batch_size + wallet_size < udl->config.wallet_persistence_batch_min_size){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a logic error: This if statement will only add wallet_queue.front() to the current batch if the new batch size (curr_batch_size + wallet_size) would be less than the minimum batch size, which is (almost) never true. It should either test if the new batch size would be greater than the minimum, or test if the new batch size would be less than the maximum. I'm not sure which is intended, but I'm guessing it's "greater than the minimum"

Comment thread src/core/cbdc_udl.cpp
}

// Reset for next batch
curr_batch_size = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this reset on every loop iteration? Shouldn't it only be reset if the batch was actually sent via put_objects (i.e. inside the above if statement?)

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.

2 participants