API endpoint for manually relaying warp message #327

geoff-vball · 2024-06-10T15:58:21Z

Why this should be merged

Closes #314

How this works

Creates a global MessageCoordinator that contains maps to Source Clients, Application Relayers, and Message Handler Factories.

TODOs:

Make block number optional (implement iterative look-back for the message)
Flag to enable/disable the API?
Documentation around potential DOS vectors of the API should it be open to the public networking wise?
Should any authentication be required? Probably makes sense to recommend it is only an RPC available locally.
Currently we have no way to distinguish between the transaction being successfully sent and ShouldSendMessage() returning false.

How this was tested

E2E Test

How is this documented

TODO

…nd block number

main/main.go

cam-schultz

I left a handful of comments and asks, and I'd like to think a bit more about the new MessageCoordinator type. But overall, the changes make sense and I'm a big fan of the trend of decoupling all of the components and coordinating them in main.

main/main.go

cam-schultz · 2024-06-13T20:34:06Z

main/main.go

-				"Created application relayers",
-				zap.String("blockchainID", sourceBlockchain.BlockchainID),
-			)
+			return relayer.ProcessManualWarpMessages(manualWarpMessages[sourceBlockchain.GetBlockchainID()])


We should add deprecation notes for manual Warp messages in the config, and perhaps emit logs when they are provided.

Sorry, I already removed the manual warp messages from the config. Do you think we should keep them in there for a few releases with a deprecation note? I can't imagine many people were using them already given the difficulty in having to add them to the nodes as well.

Yeah, we should deprecate features gracefully at this point, even though I agree that likely few people are using this particular feature. Apologies for not making that clear up front

So a couple thoughts here. We don't initialize the logger for the application until after we finish parsing the config. So it would be a little hard to emit logs from here. Returning an error and failing validation seems a little too extreme here, is there some middle ground?

Do we still want to process the manual warp messages if they're included in the config? It should be easy enough to point the users to the new API.

relayer/listener.go

relayer/relay_message_api_handler.go

cam-schultz · 2024-06-13T21:34:19Z

relayer/relay_message_api_handler.go

Let's document this endpoint in the top-level README

relayer/relay_message_api_handler.go

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

main/main.go

minghinmatthewlam · 2024-06-18T18:07:06Z

main/main.go

+) (map[ids.ID]map[common.Address]messages.MessageHandlerFactory, error) {
+	messageHandlerFactories := make(map[ids.ID]map[common.Address]messages.MessageHandlerFactory)
+	for _, sourceBlockchain := range globalConfig.SourceBlockchains {
+		messageHandlerFactoriesForSource := make(map[common.Address]messages.MessageHandlerFactory)


nit: can we rename to messageProtocolHandlerFactory or simiilar

Rename which?

messageHandlerFactoriesForSource

README.md

relayer/message_coordinator.go

README.md

api/relay_message.go

Co-authored-by: F. Eugene Aumson <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

cam-schultz

Two more final comments, then I think we're good to go

api/relay_message.go

main/main.go

api/relay_message.go

michaelkaplan13 · 2024-07-02T19:10:01Z

relayer/message_coordinator.go

+	error,
+) {
+	// Check that the warp message is from a supported message protocol contract address.
+	messageHandlerFactory, supportedMessageProtocol := mc.messageHandlerFactories[warpMessageInfo.UnsignedMessage.SourceChainID][warpMessageInfo.SourceAddress]


Nit: This line looks very long...I'm not too worried about it specifically but surprised we're not getting a linter error/warning in CI

It looks like this can be enabled with the lll linter: https://golangci-lint.run/usage/linters/#lll

Worth noting that the Go team has explicitly denied adding line length limits to gofmt.

There are a good number of lines that don't fit in the line length, so I'll make a separate ticket for this.

michaelkaplan13

Last few nits, but LGTM otherwise

cam-schultz

One last uber mega nit that I'm just noticing now - can we capitalize the first letters of the log message strings for consistency? I think the only files that need to be modified are health_check.go and relay_message.go

cam-schultz

LGTM! 🙏

bernard-avalabs

LGTM

geoff-vball added 4 commits June 7, 2024 10:40

Refactor application relayers

315049b

wip

6254a6c

Merge branch 'main' into gstuart/api-relay

021269f

Functions for relaying message from blockchain ID, warp message ID, a…

ccd4ef5

…nd block number

github-advanced-security bot found potential problems Jun 10, 2024

View reviewed changes

main/main.go Fixed Show fixed Hide fixed

geoff-vball added 10 commits June 10, 2024 12:04

lint

a6fb5cf

Fixes

d67e619

WIP

96ecc68

Merge branch 'main' into gstuart/api-relay

7af931b

Add test for API

7703831

lint

eb3e4a0

Fix test

d4042c6

Fix test

9081636

Merge branch 'main' into gstuart/api-relay

2cc6522

small fixes

818f650

geoff-vball marked this pull request as ready for review June 12, 2024 15:45

geoff-vball requested review from michaelkaplan13, cam-schultz, minghinmatthewlam and bernard-avalabs as code owners June 12, 2024 15:45

cam-schultz reviewed Jun 13, 2024

View reviewed changes

geoff-vball and others added 2 commits June 14, 2024 10:48

Add error messages

8325943

Update relayer/relay_message_api_handler.go

23a9b2a

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

cam-schultz mentioned this pull request Jun 17, 2024

Add Protocol Address to RelayerID #337

Open

geoff-vball added 2 commits June 18, 2024 10:50

Merge branch 'main' into gstuart/api-relay

a58649a

Review fixes

6dc2441

minghinmatthewlam reviewed Jun 18, 2024

View reviewed changes

main/main.go Outdated Show resolved Hide resolved

minghinmatthewlam reviewed Jun 18, 2024

View reviewed changes

main/main.go Outdated Show resolved Hide resolved

minghinmatthewlam reviewed Jun 18, 2024

View reviewed changes

main/main.go Outdated Show resolved Hide resolved

minghinmatthewlam reviewed Jun 18, 2024

View reviewed changes

geoff-vball requested a review from cam-schultz June 27, 2024 21:19

cam-schultz reviewed Jun 28, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

relayer/message_coordinator.go Outdated Show resolved Hide resolved

feuGeneA reviewed Jul 1, 2024

View reviewed changes

geoff-vball and others added 9 commits July 2, 2024 11:02

Update api/relay_message.go

570953c

Co-authored-by: F. Eugene Aumson <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Update README.md

24e7694

Co-authored-by: F. Eugene Aumson <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Update api/relay_message.go

abfb84e

Co-authored-by: F. Eugene Aumson <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Update README.md

6995b64

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Update README.md

713a80a

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Update README.md

a68f7a4

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Update README.md

4ade921

Co-authored-by: cam-schultz <[email protected]> Signed-off-by: Geoff Stuart <[email protected]>

Review fixes

5468239

Fix test

349e6c3

geoff-vball requested review from cam-schultz and feuGeneA July 2, 2024 16:10

cam-schultz reviewed Jul 2, 2024

View reviewed changes

api/relay_message.go Outdated Show resolved Hide resolved

main/main.go Outdated Show resolved Hide resolved

review fixes

343651b

geoff-vball requested a review from cam-schultz July 2, 2024 17:48

michaelkaplan13 reviewed Jul 2, 2024

View reviewed changes

api/relay_message.go Outdated Show resolved Hide resolved

michaelkaplan13 reviewed Jul 2, 2024

View reviewed changes

review fixes

b4376e2

geoff-vball requested a review from michaelkaplan13 July 2, 2024 19:40

cam-schultz reviewed Jul 2, 2024

View reviewed changes

Capitalize log messages

f33e56f

geoff-vball requested a review from cam-schultz July 2, 2024 19:46

cam-schultz approved these changes Jul 2, 2024

View reviewed changes

bernard-avalabs approved these changes Jul 3, 2024

View reviewed changes

michaelkaplan13 approved these changes Jul 3, 2024

View reviewed changes

geoff-vball merged commit e1d4d77 into main Jul 3, 2024
7 checks passed

geoff-vball deleted the gstuart/api-relay branch July 3, 2024 15:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API endpoint for manually relaying warp message #327

API endpoint for manually relaying warp message #327

geoff-vball commented Jun 10, 2024 •

edited

Loading

cam-schultz left a comment

cam-schultz Jun 13, 2024

geoff-vball Jun 25, 2024

cam-schultz Jun 25, 2024

geoff-vball Jun 26, 2024

cam-schultz Jun 13, 2024

minghinmatthewlam Jun 18, 2024

geoff-vball Jun 18, 2024

minghinmatthewlam Jun 20, 2024

cam-schultz left a comment

michaelkaplan13 Jul 2, 2024

cam-schultz Jul 2, 2024 •

edited

Loading

geoff-vball Jul 2, 2024

michaelkaplan13 left a comment

cam-schultz left a comment

cam-schultz left a comment

bernard-avalabs left a comment

API endpoint for manually relaying warp message #327

API endpoint for manually relaying warp message #327

Conversation

geoff-vball commented Jun 10, 2024 • edited Loading

Why this should be merged

How this works

How this was tested

How is this documented

cam-schultz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cam-schultz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cam-schultz Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkaplan13 left a comment

Choose a reason for hiding this comment

cam-schultz left a comment

Choose a reason for hiding this comment

cam-schultz left a comment

Choose a reason for hiding this comment

bernard-avalabs left a comment

Choose a reason for hiding this comment

geoff-vball commented Jun 10, 2024 •

edited

Loading

cam-schultz Jul 2, 2024 •

edited

Loading