Skip to content

Snap client fixes#6847

Merged
garyschulte merged 7 commits intobesu-eth:mainfrom
garyschulte:snap-client-fixes
Mar 30, 2024
Merged

Snap client fixes#6847
garyschulte merged 7 commits intobesu-eth:mainfrom
garyschulte:snap-client-fixes

Conversation

@garyschulte
Copy link
Copy Markdown
Contributor

@garyschulte garyschulte commented Mar 29, 2024

PR description

Fixed Issue(s)

snap client fixes discovered during besu snap-client -> besu snap-server testing.

  • correctly handle empty range responses
  • skip stack trie commits without keys
  • adjust remote peer connection limit arithmetic to cover cases where max-peers is 1

rip-coderpal
RIP coder buddy

try {
final boolean isEmptyRange =
(response.slots().isEmpty() || response.slots().get(0).isEmpty())
&& !response.proofs().isEmpty();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be ((A || B) && !C)
or
(A || (B & !C))

does it make sense for the response.slots to be empty if response.proofs is not empty

Copy link
Copy Markdown
Contributor Author

@garyschulte garyschulte Mar 29, 2024

Choose a reason for hiding this comment

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

this is correct as written. If there were no slots in the range besu will return no slots and a proof of exclusion 👍 .

edit: I realize on second read you are asking about order of evaluation.

Copy link
Copy Markdown
Contributor Author

@garyschulte garyschulte Mar 29, 2024

Choose a reason for hiding this comment

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

|| takes precedent over &&, so this should be ((A || B) && !C). 🙏

(which is how it is written, but it is hard to tell because it is split across lines)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

added a suggested rewrite for the comment

@garyschulte garyschulte enabled auto-merge (squash) March 29, 2024 21:25
@garyschulte garyschulte disabled auto-merge March 29, 2024 21:47
@garyschulte garyschulte enabled auto-merge (squash) March 29, 2024 21:49
@garyschulte garyschulte disabled auto-merge March 29, 2024 21:49
@garyschulte garyschulte enabled auto-merge (squash) March 29, 2024 21:52
matkt and others added 7 commits March 29, 2024 16:26
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
…still can accept remote connections

Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte merged commit deaea9b into besu-eth:main Mar 30, 2024
@garyschulte garyschulte deleted the snap-client-fixes branch April 3, 2024 19:28
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* manage empty range for storage
* round rather than floor on max remote connections so that maxpeers=1 still can accept remote connections
---------

Signed-off-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Karim Taam <karim.t2am@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: amsmota <antonio.mota@citi.com>
amsmota pushed a commit to Citi/besu that referenced this pull request Apr 16, 2024
* manage empty range for storage
* round rather than floor on max remote connections so that maxpeers=1 still can accept remote connections
---------

Signed-off-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Karim Taam <karim.t2am@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: amsmota <antonio.mota@citi.com>
matthew1001 pushed a commit to kaleido-io/besu that referenced this pull request Jun 7, 2024
* manage empty range for storage
* round rather than floor on max remote connections so that maxpeers=1 still can accept remote connections
---------

Signed-off-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Karim Taam <karim.t2am@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
jflo pushed a commit to jflo/besu that referenced this pull request Jun 7, 2024
* manage empty range for storage
* round rather than floor on max remote connections so that maxpeers=1 still can accept remote connections
---------

Signed-off-by: garyschulte <garyschulte@gmail.com>
Co-authored-by: Karim Taam <karim.t2am@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
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