-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Aptos LOOP setup #14076
Aptos LOOP setup #14076
Conversation
I see you updated files related to
|
// the plugin should handle it's own defaults and merging | ||
c.Aptos = f.Aptos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unfortunately not so simple, because we also support passing multiple files that override one another. I can't think of a simple short-term fix, but this will have to be solved another way eventually regardless 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you concat the files together, then only do the parse once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plain concat only works in special cases. We could write logic to crawl through the maps and set individual fields, but I'm not sure how easy that will be 🤔
Blake2b costs about a third of SHA2/SHA3/keccak256 https://aptos.dev/en/build/smart-contracts/cryptography
Really painful on slower internet connections
TOMLConfigs RawConfigs | ||
} | ||
|
||
func (r *RelayerFactory) NewAptos(ks keystore.Aptos, chainCfgs RawConfigs) (map[types.RelayID]loop.Relayer, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: shouldn't we use the AptosFactoryConfig
as input param here?
@@ -301,3 +301,65 @@ func (r *RelayerFactory) NewCosmos(config CosmosFactoryConfig) (map[types.RelayI | |||
} | |||
return relayers, nil | |||
} | |||
|
|||
type AptosFactoryConfig struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[non-blocking discussion] We should also ideally get Beholder integrated and configured in the core Node itself, and then inject it here to make it available to the LOOP as a common core service. @mathewdgardner wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Draft in progress here to follow beholder client PR: smartcontractkit/chainlink-common#696
return r.NewLOOPRelayer("Aptos", corerelay.NetworkAptos, plugin, loopKs, chainCfgs) | ||
} | ||
|
||
func (r *RelayerFactory) NewLOOPRelayer(name string, network string, plugin env.Plugin, ks coretypes.Keystore, chainCfgs RawConfigs) (map[types.RelayID]loop.Relayer, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider grouping the keystore, config, and capRegistry in a type like PluginContext
@@ -88,6 +85,9 @@ func (akr *aptosKeyring) verifyBlob(pubkey ocrtypes.OnchainPublicKey, b, sig []b | |||
if len(sig) != akr.MaxSignatureLength() { | |||
return false | |||
} | |||
// we ignore the passed in publicKey since that will be the EVM key if using multi-chain config | |||
// instead we use the embedded publicKey that's part of the ed25519 signature | |||
pubkey = sig[:32] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this feels like magic. Shouldn't the caller configure this on input?
Quality Gate passedIssues Measures |
No description provided.