Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: add paging to utxo stream request #5302

Merged
merged 3 commits into from Apr 12, 2023

Conversation

SWvheerden
Copy link
Collaborator

@SWvheerden SWvheerden commented Apr 11, 2023

Description

Adds paging to the utxo stream to handle the large amount of faucet utxos

Motivation and Context

See: #5299

How Has This Been Tested?

unit tests

Fixes: #5299

What process can a PR reviewer use to test or verify this change?

See PR #5298

Breaking Changes

Changes request service

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

@SWvheerden
Copy link
Collaborator Author

This is an alternative to PR #5298

The issue we have is that the Genesis block has too many utxos to send over the wire with the 4mb size limit.
This should only be an issue with the Genesis block as with any other block the whole block should be less than 4mb.
We can increase the size limit, but then increase the size for everybody. This slightly ever increases the send/receive code to handle streaming of sub block batches of utxos.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Apr 11, 2023
@SWvheerden SWvheerden changed the title feat!: add paging to request feat!: add paging to utxo stream request Apr 11, 2023
hansieodendaal
hansieodendaal previously approved these changes Apr 11, 2023
Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

utACK

// Ensure task stops if the peer prematurely stops their RPC session
if tx.send(Ok(utxo_block_response)).await.is_err() {
break;
for utxo_chunk in utxos.chunks(2000) {
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 safely increase this number to 4000 as 5526 UTXOs in the faucet takes up approximately 4.5924 MiB. Thus (4000 / 5526) x 4.5924 = 3.3242 MiB.

@ghpbot-tari-project ghpbot-tari-project removed P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Apr 11, 2023
hansieodendaal
hansieodendaal previously approved these changes Apr 11, 2023
Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

utACK

Please see comment about for utxo_chunk in utxos.chunks(2000)

@stringhandler stringhandler merged commit 3540309 into tari-project:development Apr 12, 2023
9 checks passed
SWvheerden added a commit that referenced this pull request Apr 12, 2023
##
[0.50.0-pre.0](v0.49.0-pre.6...v0.50.0-pre.0)
(2023-04-12)


### ⚠ BREAKING CHANGES

* add paging to utxo stream request (5302)

### Features

* add extended mask recovery
([5301](#5301))
([23d882e](23d882e))
* add network name to data path and --network flag to the miners
([5291](#5291))
([1f04beb](1f04beb))
* add other code template types
([5242](#5242))
([93e5e85](93e5e85))
* add paging to utxo stream request
([5302](#5302))
([3540309](3540309))
* add wallet daemon config
([5311](#5311))
([30419cf](30419cf))
* define different network defaults for bins
([5307](#5307))
([2f5d498](2f5d498))
* feature gates
([5287](#5287))
([72c19dc](72c19dc))
* fix rpc transaction conversion
([5304](#5304))
([344040a](344040a))
@SWvheerden SWvheerden deleted the sw_paging_request branch May 3, 2023 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wallet can request messages that are over the size limit for rpc messages
4 participants