Xrpl ntt accountant by douglasgalico · Pull Request #4893 · wormhole-foundation/wormhole

douglasgalico · 2026-07-06T17:32:44Z

Integrates XRPL into the NTT global accountant. The XRPL watcher now recognizes NTT transfer emitters and synthesizes transceiver registration messages, so XRPL custody accounts can be tracked by the ntt-global-accountant

evan-gray

I might be misunderstanding something, but between this and the sequencer PR, I'm not clear how the XRPL manager sends the payment to the XRPL core bridge account. I'm also concerned about not being able to accurately distinguish locking and burning configurations.

evan-gray · 2026-07-07T12:23:15Z

+	out := make([]byte, 0, transceiverInfoLen)
+	out = append(out, transceiverInfoPrefix[:]...)
+	out = append(out, manager32[:]...)
+	out = append(out, nttModeLocking)


why is this always locking? an XRPL manager could be either locking or burning depending on if it is the asset issuer.

evan-gray · 2026-07-07T12:28:21Z

+[4]byte  prefix = "XREG"
+uint8    kind              // 0 = Hub, 1 = Peer
+[20]byte manager           // XRPL custody account id
+[]byte   token_id          // wire form: XRP 1B / IOU 41B / MPT 25B


I assume this follows the encoding from the sequencer spec?

token_id encoding:

Token Type Byte 103 Bytes 104+ Total token_id

XRP 0x00 (none) 1 byte

IOU 0x01 currency (20) + issuer (20) 41 bytes

MPT 0x02 mpt_issuance_id (24) 25 bytes

evan-gray · 2026-07-07T12:29:25Z

+```go
+[4]byte  prefix = 0x9c23bd3b
+[32]byte manager_address    // manager32 (left-padded XRPL account)
+uint8    manager_mode = 1   // locking


where is the mode determined?

evan-gray · 2026-07-07T12:31:34Z

+<!-- cspell:ignore XREG vaas -->

-> 🚧 TODO
+### Registration (Initialize Transceiver & Peer Registration)


Might be worth linking to https://github.com/wormhole-foundation/native-token-transfers/blob/main/docs/Transceiver.md in this section

evan-gray · 2026-07-07T12:36:39Z

-### Transceiver (Peer) Registration
+Because XRPL has no smart contracts, the `WormholeTransceiverInfo` (hub init) and
+`WormholeTransceiverRegistration` (peer) messages cannot be emitted natively.
+Instead, the Solana Sequencer emits an `XrplRegistration` (prefix `XREG`) message


I might have expected the Solana Sequencer to form the payload and the core bridge to allow for a generic "emit as transceiver" functionality that accepts a token_id and generic payload. Though I suppose this approach prevents malicious transfer messages from being emitted directly via the core bridge so perhaps it is more secure albeit more tedious (and would need to be extended across multiple codebases should the need for another message type arise).

evan-gray · 2026-07-07T12:54:41Z

+	xregTokenXRP   = 0x00 // wire token_id opening byte for XRP, mirrors XrplTokenId
+	xregTokenIOU   = 0x01 // wire token_id opening byte for issued currencies
+	xregTokenMPT   = 0x02 // wire token_id opening byte for MPT
+	nttModeLocking = 0x00 // manager mode for XRPL custody accounts (locking; accountant only accepts locking)


accountant only accepts locking hubs, which is true, but these init messages should reflect whether the manager is in locking or burning mode. all XRPL managers should not have messages attesting that they are locking when they may not be.

Added logic to detect based on token_id (IOU and MPT) by checking if Issuer is the same as the Manager/CustodyAccount

evan-gray · 2026-07-07T13:03:21Z

+message under the correct NTT transceiver emitter.

-> 🚧 TODO
+Upon a Payment to the Core Account, the guardian watcher MUST


this also must verify that the message came from the matching manager? though I'm confused about how an XREG message from the sequencer results in a payment from the manager without either manager changes in this PR or a generic payment message in the Sequencer PR. I would expect the latter - that the Sequencer issues a generic payment to the core bridge containing this registration. Maybe I'm missing where that occurs, but it looks like this is simply posted as a VAA payload. 😕

Changed at sequencer level! XREG is wrapped in XREL which is signed by delegated manager and captured by watcher.

evan-gray · 2026-07-07T13:17:22Z

+// managed — used to authenticate an XREG registration publish (see the SECURITY
+// note in parseRegistrationTransaction). A missing/malformed Account field is an
+// error; an unmanaged sender is a silent decline (nil, nil).
+func (p *Parser) registrationSenderAccountID(tx transaction.FlatTransaction) ([]byte, error) {


we seem to have this code multiple times now - this is very similar to extractSender

evan-gray · 2026-07-07T13:20:19Z

+
+	// manager32 = left-padded 20-byte account id (same as transfer sourceNTTManager).
+	var manager32 [32]byte
+	copy(manager32[12:], manager20[:])


getting nervous about the multiple re-implementations in this file - there's logic for this in addressToEmitter but they don't quite match.

Note that common operations like this are often covered and well-tested by functions in the Go SDK. (under sdk/)

evan-gray · 2026-07-07T13:23:43Z

+	}
+	tail := data[xregHeaderLen+consumed:]
+
+	emitter = (&Parser{}).calculateEmitterAddress(manager32, sourceToken)


not sure calculateEmitterAddress needs to be method of parser, maybe switch it to a free function instead or match the other helpers and make buildRegistrationMessage a method of Parser.

bemic · 2026-07-14T22:22:25Z


+	// Registration (XREG) publishes go to the core account but use a distinct
+	// MemoFormat, so they must be checked before the generic core parser.
+	msg, err = p.parseRegistrationTransaction(tx)


If the XREG is wrapped as XREL and send as standard payment, the transaction should produce XACK.
In that case this code won't be reached because parseXACKTransaction will consume the tx.

The test did not catch this because the regTxStream function builds the transaction without the TicketSequence. In such case parseXACKTransaction returns (nil, nil) because the field is empty and parseRegistrationTransaction takes place. However the manager code assigns TicketSequence: uint32(payload.TicketID).

Thanks for spotting it! Indeed it's a miss, parser was adjusted to output a list of transactions (the registration message for accountant and ACK) - adjusted also at watcher code in all paths

mdulin2 · 2026-07-14T18:27:20Z

 //  2. parseXACKTransaction — ticket-consuming transactions on a managed account
 //     (Release Payments, failed TicketCreate, Burn/AccountSet) produce an XACK.
 //  3. parseCoreTransaction — payments to the core account with a Wormhole core
 //     memo produce a generic Wormhole message.


Missing comment for 'parseRegistrationTransaction' on the flow of this. Since we do it for the other four, I'd recommend adding it here.

mdulin2 · 2026-07-14T22:34:53Z

 		return msg, err
 	}

+	// Registration (XREG) publishes go to the core account but use a distinct


The XREG is being sent through the manager, which uses tickets. This currently doesn't handle the ticket management because it's not sending an ACK back to the sequencer. This means that the ticket would effectively be stuck there until a BURN was triggered for the ticket. Naturally, this would fail because the sequence was indeed used up.

Additionally, what should we do in the case that the XREG fails? Should the hub or peer registration accountant message be rebroadcasted?

Fixed the ordering of checks in parser and ACK is handled properly. For the XREG failure there was also a gap, now after checking the transaction result - and when it fails - it will pass through parseXACKTransaction, with that we can consume XACK and clear the ticket on sequencer.

johnsaigle · 2026-07-15T10:23:36Z

+var transceiverRegPrefix = [4]byte{0x18, 0xfc, 0x67, 0xc2}  // WormholeTransceiverRegistration
+
+// XREG constants
+const (


suggestion: Could we move the XRPL discriminator constants into custom typed domains instead of leaving them as untyped constants?

Right now values like xRegKind and xRegTokenXRP are easy to mix in-place because the compiler does not know they represent different concepts. A future change could accidentally compare or pass a registration kind where a token kind is expected, and it would still compile if both are just plain constants.

It would look something like this:

type xRegKind string type xRegToken string const ( xRegKindRegister xRegKind = "..." xRegKindAck xRegKind = "..." xRegTokenXRP xRegToken = "..." )

Then helper signatures can encode intent:

func buildXRegPayload(kind xRegKind, token xRegToken) ...

That makes invalid mixing harder to write and easier to catch during review/compile time.

I’d also suggest putting these XRPL domain types and constants in a dedicated file, e.g. node/pkg/watchers/xrpl/types.go, so the parser code stays focused on transaction handling and the XRPL wire/domain vocabulary has one obvious home.

Yep! Moved the XREG wire consts into a dedicated types.go file. Kept them untyped to match the package other wire discriminants (tokenType*, xackTxType*, ..) since they are raw bytes read from/appended to []byte

mdulin2 · 2026-07-15T16:45:32Z

+		return nil, nil
+	}
+
+	destination, err := p.extractDestination(tx.Transaction)


This returning an error is a problem with the current ordering again. There are many transaction types where the destination is NOT set. Both a failed TicketCreate and AccountSet (burn ticket) will hit this error, and return, skipping the parseXACKTransaction logic.

I think we need more tests to ensure things work with the current ordering realistic RPC responses, and realistic settings. This is the second change that would have broken existing functionality in the watcher within this PR.

From what I can tell, the reason the existing tests didn't catch this was because the coreAccount was never set on the tests, hitting the exit path with no error above this. In reality, the existing tests (if properly configured) should have caught this regression.

johnsaigle · 2026-07-15T18:32:41Z


 // fetchAndParseTransaction fetches a specific transaction by hash for reobservation.
-func (w *Watcher) fetchAndParseTransaction(txHash []byte) (*common.MessagePublication, error) {
+func (w *Watcher) fetchAndParseTransaction(txHash []byte) ([]*common.MessagePublication, error) {


suggestion: update the function name and comment to clarify that this now returns multiple message publications

johnsaigle · 2026-07-15T18:40:36Z

+// registration consumed on the sequencer. Every other transaction yields at most
+// one message.
+//
+// Returns an empty slice if none matched.


Suggested change

// Returns an empty slice if none matched.

// Returns nil if none matched.

The function only returns nil or non-empty slices, not an empty slice.

johnsaigle · 2026-07-15T18:51:03Z

-	var emitter vaa.Address
-	copy(emitter[32-addresscodec.AccountAddressLength:], accountID)
+	// Right-align the 20-byte account id in a 32-byte vaa.Address (left-padded
+	// with 12 zero bytes). Uses the SDK helper rather than hand-rolled copying.


Suggested change

// with 12 zero bytes). Uses the SDK helper rather than hand-rolled copying.

// with 12 zero bytes).

This line can be removed -- I assume this was added during an LLM revision but we want to avoid comments like this getting into the codebase

johnsaigle · 2026-07-15T18:52:08Z

 // calculateEmitterAddress calculates the emitter address from source NTT manager and source token.
 // emitter = keccak256("ntt" + source_ntt_manager_address + source_token)
-func (p *Parser) calculateEmitterAddress(sourceNTTManager, sourceToken [32]byte) vaa.Address {
+// It is a pure function of its inputs (no Parser state), so it is a free function.


This line can be removed -- I assume this was added during an LLM revision but we want to avoid comments like this getting into the codebase

johnsaigle · 2026-07-15T18:59:13Z

+// one message.
+//
+// Returns an empty slice if none matched.
+func (p *Parser) parseTransaction(tx GenericTx) ([]*common.MessagePublication, error) {


Given how complicated this function is, I think it's worth threading a zap.Logger through to the Parser type. It could just use a pointer to the logger the watcher already has.

This would allow us to add logging alongside the detection of the transaction type. It would be helpful for testing and debugging to know if this function ever returns the wrong type, for example.

douglasgalico requested review from SEJeff, bemic, djb15, evan-gray, johnsaigle, kcsongor, mdulin2, panoel and pleasew8t as code owners July 6, 2026 17:32

M-Picco previously approved these changes Jul 6, 2026

View reviewed changes

evan-gray requested changes Jul 7, 2026

View reviewed changes

douglasgalico dismissed M-Picco’s stale review via fe5de8e July 9, 2026 10:48

bemic reviewed Jul 14, 2026

View reviewed changes

mdulin2 requested changes Jul 14, 2026

View reviewed changes

johnsaigle reviewed Jul 15, 2026

View reviewed changes

Comment thread node/pkg/watchers/xrpl/README.md Outdated

Comment thread node/pkg/accountant/ntt_config.go

johnsaigle mentioned this pull request Jul 15, 2026

XRPL manager and watcher should support NTT accountant onboarding #4830

Open

douglasgalico added 7 commits July 15, 2026 17:09

feat: XRPL NTT Accountant

b5dfe08

chore: authenticate XREG registration sender against managed accounts

206df42

fix: tests

9b8e0c5

feat: NTT Manager mode locking auto-discovery for XRPL

13be1c7

refactor: dedupe emitter/sender helpers in registration parsing

d94f6c0

fix: spellcheck ignore

b0ee086

fix: emit registration + XACK for ticketed XREG in XREL payment

e02a142

douglasgalico force-pushed the xrpl-ntt-accountant branch from 0e59a5d to e02a142 Compare July 15, 2026 15:09

douglasgalico added 5 commits July 15, 2026 17:12

fix: document parseRegistrationTransaction in the dispatch order

c84da93

fix: ack the ticket on a failed registration Payment

e65bef5

refactor: move XREG wire constants to types.go

7d5eb05

chore: add vaas/XREG/XREL to cspell-custom-words

1f9f7e4

fix: avoid err shadow in parseTransaction registration branch

6569c4e

mdulin2 reviewed Jul 15, 2026

View reviewed changes

johnsaigle reviewed Jul 15, 2026

View reviewed changes

Token Type	Byte 103	Bytes 104+	Total token_id
XRP	0x00	(none)	1 byte
IOU	0x01	currency (20) + issuer (20)	41 bytes
MPT	0x02	mpt_issuance_id (24)	25 bytes

	// Returns an empty slice if none matched.
	// Returns nil if none matched.

	// with 12 zero bytes). Uses the SDK helper rather than hand-rolled copying.
	// with 12 zero bytes).

Uh oh!

Conversation

douglasgalico commented Jul 6, 2026

Uh oh!

evan-gray left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdulin2 Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdulin2 Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mdulin2 Jul 15, 2026 •

edited

Loading

mdulin2 Jul 15, 2026 •

edited

Loading