Fetch warp numerator (rebased off of off-chain-warp-messages)
#157
Conversation
cam-schultz
requested review from
michaelkaplan13,
minghinmatthewlam,
gwen917,
geoff-vball and
bernard-avalabs
as code owners
config/config.go
Outdated
|
|
||
| // Fetch the Warp quorum values for each source subnet | ||
| for _, sourceSubnet := range c.SourceSubnets { | ||
| blockchainID, err := ids.FromString(sourceSubnet.BlockchainID) |
There was a problem hiding this comment.
Just want to make sure we're not missing a corner case here. My understanding is that each subnet can have multiple blockchains, but in the relayer code we treat every subnet as if it only contains one blockchain (though this is probably true for almost all subnets).
Let's say a subnet has two blockchains, are they supposed to have them in the config as two different "subnets"?
There was a problem hiding this comment.
Yup, blockchains are the distinct unit in the relayer config, so multiple blockchains in the same subnet would need their own distinct entries in the config.
There was a problem hiding this comment.
So probably not something for this PR, but I think that means the slices in the config should be SourceBlockchains and DestinationBlockchains. I think we're fine for now, but I can definitely see this causing confusion for anyone else who comes along in the future (and anyone consuming/forking this repo)
There was a problem hiding this comment.
Definitely agree. This is an artifact of some of the language that was used in early Warp designs, when we were considering alternative approaches than Warp to achieve inter-blockchain communication in the same subnet. I'll create a ticket to clean this up.
There was a problem hiding this comment.
Ticket here: #170
config/config.go
Outdated
|
|
||
| // Fetch the Warp quorum values for each source subnet | ||
| for _, sourceSubnet := range c.SourceSubnets { | ||
| blockchainID, err := ids.FromString(sourceSubnet.BlockchainID) |
There was a problem hiding this comment.
So probably not something for this PR, but I think that means the slices in the config should be SourceBlockchains and DestinationBlockchains. I think we're fine for now, but I can definitely see this causing confusion for anyone else who comes along in the future (and anyone consuming/forking this repo)
cam-schultz
dismissed stale reviews from michaelkaplan13 and geoff-vball
The base branch was changed.
| @@ -427,7 +429,7 @@ func (r *Relayer) RelayMessage(warpLogInfo *vmtypes.WarpLogInfo, storeProcessedH | |||
| } | |||
|
|
|||
| // Create and run the message relayer to attempt to deliver the message to the destination chain | |||
| messageRelayer := newMessageRelayer(r, unsignedMessage, destinationBlockchainID) | |||
| messageRelayer, err := newMessageRelayer(r, unsignedMessage, destinationBlockchainID) | |||
There was a problem hiding this comment.
Looks like this err check below was unnecessary previously. Glad it's corrected now 👍
Why this should be merged
Fixes #8
How this works
Fetches the Warp Quorum from the Warp Config on startup. Hardcodes the quorum value for the primary network, since that will never change.
How this was tested
Added unit tests
How is this documented
N/A