Skip to content

fix noise 2#26

Closed
Fi3 wants to merge 0 commit intoSjors:2023/11/sv2-pollfrom
Fi3:FixNoise2
Closed

fix noise 2#26
Fi3 wants to merge 0 commit intoSjors:2023/11/sv2-pollfrom
Fi3:FixNoise2

Conversation

@Fi3
Copy link

@Fi3 Fi3 commented Dec 14, 2023

  • Remove 2 bytes header from hanshake messages
  • Split noise payload in 65Kb chunks

@Sjors
Copy link
Owner

Sjors commented Dec 14, 2023

Thanks, I'll wait for your second todo item.

@Sjors
Copy link
Owner

Sjors commented Dec 15, 2023

I'm not sure if you already rebased, but try the following to get rid of my commits in the PR:

# assuming 1 commit from you
git rebase HEAD~1 --onto Sjors/2023/11/sv2-poll

@Fi3 Fi3 changed the title [WIP] fix noise 2 fix noise 2 Dec 17, 2023
@Sjors
Copy link
Owner

Sjors commented Dec 18, 2023

I changed a few things before merging:

  • std::span -> Span (see src/span.h, though this might change again with the move to c++20)
  • added a constant INITIATOR_EXPECTED_HANDSHAKE_MESSAGE_LENGTH = 170

I tested against your stratum branch stratum-mining/stratum@dev...Fi3:stratum-1:UpdateNoise though only on my custom signet, which generally doesn't have long messages.

I'll test on mainnet later.

I guess I should run an inscription tool to get >65KB messages on signet :-)

I also plan to add some more length check asserts, and rename SendMsg in noise.h to WriteMsg, to prevent confusion between creating the message and actually sending it.

@Sjors
Copy link
Owner

Sjors commented Dec 18, 2023

Changes squashed into 945838fca264c8469e24d629a4552741dbbd4dda and 945838fca264c8469e24d629a4552741dbbd4dda.

Can you take a look at key_tests.cpp, as this now fails with memory access violation at address: 0x1: no mapping at fault address around line 35. Not sure if that's because of the changes here or because of my rebase. I'll also take a look later.

@Fi3
Copy link
Author

Fi3 commented Dec 18, 2023

To tested it on testnet, with jdc you get pretty long messages. Also to test it I change the max noise chunk size to smaller value like 120 bytes and it worked.

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