Page MenuHomePhabricator

[avalanche] sign avaresponse
ClosedPublic

Authored by deadalnix on Mon, Mar 23, 01:18.

Details

Summary

While we ultimately want to use an encrypted and authenticated tunel for avalanche, such as QUIC, having a way to experiment over TCP before this is in place is useful.

TCP is not authenticated, so we need avalanche to sign all messages to do so.

Depends on D5528 and D5536

Test Plan

Updated integration tests to check the signatures.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Mon, Mar 23, 01:18
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Mar 23, 01:18
Fabien requested changes to this revision.Mon, Mar 23, 19:53
Fabien added a subscriber: Fabien.

Couple nits, but looks good.

src/Makefile.am
290 ↗(On Diff #17108)

Nit: sort

test/functional/test_framework/mininode.py
29 ↗(On Diff #17108)

Please keep the previous as a comment to indicate it's not dead code but kept for the future

This revision now requires changes to proceed.Mon, Mar 23, 19:53
jasonbcox requested changes to this revision.Wed, Mar 25, 18:49
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/test_framework/messages.py
841 ↗(On Diff #17169)

__slots__

1459 ↗(On Diff #17169)

__slots__

This revision now requires changes to proceed.Wed, Mar 25, 18:49
nakihito requested changes to this revision.Wed, Mar 25, 20:21
nakihito added a subscriber: nakihito.
nakihito added inline comments.
src/rpc/avalanche.cpp
16–18 ↗(On Diff #17169)
RPCHelpMan{"getavalanchekey, "\nReturns the key used to sign avalanche messages.\n", {}}.ToString() +

in light of D5548.

deadalnix updated this revision to Diff 17187.Thu, Mar 26, 00:22

Address comments

nakihito accepted this revision.Thu, Mar 26, 00:26

RPC help looks good.

jasonbcox accepted this revision.Thu, Mar 26, 01:06
jasonbcox added inline comments.
src/avalanche.cpp
202 ↗(On Diff #17187)

I'm seeing the Schnorr sig size scattered throughout the code and tests. It's coming time to consolidate that into a constant, though it doesn't need to be with this patch.

src/rpc/avalanche.cpp
16–18 ↗(On Diff #17169)

Nit: This message could include a note indicating that this key is subject to change. There's no guarantee that the key will be the same on successive runs.

deadalnix added inline comments.Thu, Mar 26, 01:39
src/rpc/avalanche.cpp
16–18 ↗(On Diff #17169)

There is, unless you reboot the server.

Fabien accepted this revision.Thu, Mar 26, 09:40
This revision is now accepted and ready to land.Thu, Mar 26, 09:40
This revision was automatically updated to reflect the committed changes.