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

tatanka/client: separate network parts from mesh protocol parts #3176

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

buck54321
Copy link
Member

Separate the networking logic from the mesh protocol logic by splitting the old TankaClient into two parts. MeshConn handles connections to both tatanka nodes and peers. Mesh handles the client mesh protocol. Mesh is somewhat similar to core.dexConnection in its purpose.

Here's a taste of the separated APIs.

type TheConnectionPart interface {
	ConnectPeer(peerID tanka.PeerID) error
	RequestPeer(peerID tanka.PeerID, msg *msgjson.Message, thing any) (*mj.TankagramResult, error)
	Auth(tatankaID tanka.PeerID) error
	RequestMesh(msg *msgjson.Message, thing any, opts ...RequestOption) error
}

type TheMeshFunPart interface {
	ID() tanka.PeerID
	Next() <-chan any
	Broadcast(topic tanka.Topic, subject tanka.Subject, msgType mj.BroadcastMessageType, thing interface{}) error
	PostBond(bond *tanka.Bond) error
	ActiveBonds() ([]*tanka.Bond, error)
	SubscribeMarket(baseID, quoteID uint32) error
}

Mesh has its own database. That is where things like order info and bonds will be stored. I've implemented a bonds table to demonstrate.

@buck54321 buck54321 changed the title tatanka: separate network parts from mesh protocol parts tatanka/client: separate network parts from mesh protocol parts Feb 4, 2025
Copy link
Collaborator

@martonp martonp left a comment

Choose a reason for hiding this comment

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

Definitely makes sense to separate like this.

}

// iterate iterates a table or index.
func (db *DB) iterate(keyPfix keyPrefix, table *Table, io iteratorOpts, isIndex bool, prefixI IndexBucket, f func(*Iter) error, iterOpts ...IterationOption) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func (db *DB) iterate(keyPfix keyPrefix, table *Table, io iteratorOpts, isIndex bool, prefixI IndexBucket, f func(*Iter) error, iterOpts ...IterationOption) error {
func (db *DB) iterate(keyPfix keyPrefix, table *Table, defaultOpts iteratorOpts, isIndex bool, prefixI IndexBucket, f func(*Iter) error, iterOpts ...IterationOption) error {

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a value, so it's copied on the way in. We work on the copy internally, so it wouldn't make sense to name it "default" any more.

Comment on lines +194 to +210
// UseDefaultIterationOptions sets default options for Iterate.
func (t *Table) UseDefaultIterationOptions(optss ...IterationOption) {
for i := range optss {
optss[i](&t.defaultIterationOptions)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused.

if err != nil {
return err
}
enc, err := p.encryptRSA(b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is already existing code, but what's the reason for having a separate RSA key pair in addition to the ECDSA one?

Copy link
Member

@JoeGruffins JoeGruffins Feb 18, 2025

Choose a reason for hiding this comment

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

I'm also confused about the signing patterns.

If we just use ws/wss we wouldn't need to sign all messages to prove who sent them, right? wss would encrypt.

If the purpose is to prove to a third party that this message was sent, then we use the signature from the private key of the secp256k1.PublicKey that is unique for every user.

Copy link
Member Author

@buck54321 buck54321 Feb 20, 2025

Choose a reason for hiding this comment

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

The RSA keypair is for encryption/decryption of p2p messages. This is end-to-end encryption. Server can't read the messages.

If we just use ws/wss we wouldn't need to sign all messages to prove who sent them, right?

Right. For WSS connections, only the initial message needs to be signed now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another option worth considering instead of RSA is AES for symmetric encryption. Both peers would use the same key, derived via ECDH using the user’s private key and the counterparty’s public key (e.g., existing secp256k1 keys from DCRDEX identities). This skips a separate key exchange since we’re leveraging keys we already have, and AES is way faster than RSA, microseconds versus milliseconds per message. Here's how to derive an AES key:

func deriveAESKey(privKey *btcec.PrivateKey, peerPubKey *btcec.PublicKey) ([]byte, error) {
	// ECDH: Compute shared secret
	sharedX, _ := privKey.ECDH(peerPubKey)
	sharedSecret := sharedX.Bytes() // x-coordinate (32 bytes)

	// HKDF: Derive AES-256 key
	hkdf := hkdf.New(sha256.New, sharedSecret, nil, nil)
	aesKey := make([]byte, 32) // 256 bits
	_, err := hkdf.Read(aesKey)
	if err != nil {
		return nil, fmt.Errorf("failed to derive AES key: %v", err)
	}
	return aesKey, nil
}

Comment on lines +300 to +302
func (m *Mesh) ConnectPeer(peerID tanka.PeerID) error {
return m.conn.ConnectPeer(peerID)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this only exposed for testing? Looks like right now you connect to a new peer other than the entry node when a message arrives from that peer. When connecting to the entry node, the entry node could share all of its peers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Uh huh. I'm not certain yet how the server node connection scheme is going to shake out. Will we connect to all the tatanka nodes that we can, or just one at a time and use the others as backups should ours go offline? When we broadcast a message, we should only send it to one, right? How about p2p messaging? Is there some kind of path optimization to be done?

And yes, this is only exposed for testing purposes for right now.

type TatankaSelectionMode string

const (
SelectionModeEntryNode TatankaSelectionMode = "EntryNode"
Copy link
Collaborator

Choose a reason for hiding this comment

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

With what kind of request would you only want to message the entry node?


// Config is the configuration for the MeshConn.
type Config struct {
EntryNode *TatankaCredentials
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why only one EntryNode rather than a list?

Comment on lines 346 to 348
if err := cm.Connect(ctx); err != nil {
c.log.Errorf("error connecting to tatanka node %s at %s: %v. will keep trying to connect", err)
}
Copy link
Member

Choose a reason for hiding this comment

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

If there is an error that would be returned from the websocket the ctx is canceled and this won't keep trying I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. We'll have to look into reconnect handling too.

Comment on lines 407 to 414
for peerID, tt := range c.tatankaNodes {
if tt.cm.On() && peerID != skipID {
tts = append(tts, tt)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can return earlier if Any.

if err != nil {
return err
}
enc, err := p.encryptRSA(b)
Copy link
Member

@JoeGruffins JoeGruffins Feb 18, 2025

Choose a reason for hiding this comment

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

I'm also confused about the signing patterns.

If we just use ws/wss we wouldn't need to sign all messages to prove who sent them, right? wss would encrypt.

If the purpose is to prove to a third party that this message was sent, then we use the signature from the private key of the secp256k1.PublicKey that is unique for every user.

c.handlers.HandleTatankaNotification(tatankaID, msg)
return nil
default:
c.log.Errorf("tatanka node %s send a message with an unhandleable type %d", msg.Type)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
c.log.Errorf("tatanka node %s send a message with an unhandleable type %d", msg.Type)
c.log.Errorf("tatanka node %s send a message with an unhandleable type %d", tt.peerID, msg.Type)

@martonp
Copy link
Collaborator

martonp commented Mar 1, 2025

I’ve updated the encryption to use AES instead of RSA in this diff: 03c89e7

It retains the connection request to establish an ephemeral key pair for each peer connection. This ensures that if a permanent private key leaks, past and future communications remain secure due to the ephemeral keys.

I suggest adding a typed TankagramResultPayload with an error message field. This would allow peers to signal issues—like when one restarts, loses its ephemeral key pair, and the counterparty tries to use the old pair. I held off on implementing this to minimize merge conflicts for now.

Separate the networking logic from the mesh protocol logic by
splitting the old TankaClient into two parts. MeshConn handles
connections to both tatanka nodes and peers. Mesh handles the
client mesh protocol.

Mesh has its own database. That is where things like order info
and bonds will be stored.
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