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

feat(monitor): add xcall loadgen to the monitor app #2723

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kc1116
Copy link
Contributor

@kc1116 kc1116 commented Dec 19, 2024

This PR adds xcall load generation to the monitor app.

issue: #2275

- implement GetPortal func on xconnector
- add xcaller and config
- add xcall forever
- enable xcaller in devnet1 manifest
@kc1116 kc1116 changed the title feat(monitor): Add xcall loadgen to the monitor app feat(monitor): add xcall loadgen to the monitor app Dec 19, 2024
@kc1116 kc1116 marked this pull request as ready for review December 23, 2024 14:37
@@ -131,7 +131,7 @@ func (s Static) ConsensusRPC() string {
if s.Network == Devnet {
// First halo in devnet docker-compose.
// Note that it might not be running.
return "http://localhost:5701"
return "http://validator01:26657"
Copy link
Contributor

Choose a reason for hiding this comment

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

lib/netconf/static.go should remain external RPCs. which for devnet, are local host

with have internalEndpoints() in e2e to get endpoints from within a network (docker or gcp)

Comment on lines 72 to +83
[loadgen]
# Validator keys glob defines the validator keys to use for self-delegation.
validator-keys-glob = "path/*/1"

[xcaller]
enabled = "false"

# Chain ID pairs, where each pair specifies a 'from' and 'to' chain for sending xCalls.
[xcaller.chainid-pairs]
# arbitrum_one = "optimism"
# base = "arbitrum_one"

Copy link
Contributor

Choose a reason for hiding this comment

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

xcaller is part of loadgen, so let's make it part of loadgen config

xCallerAddr := eoa.MustAddress(network.ID, eoa.RoleXCaller)
go xCallForever(ctx, xCallerAddr, period, xCallerCfg.ChainIDs, backends, connector)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This Start function has a lot of stuff in it now. First half about loadgening validator updates. Second part about loadgening xcalls. Maybe put both in their own function

Comment on lines +8 to +18
xcaller_enabled = true

[node.validator01]
[node.validator02]

[node.fullnode01]
mode="archive"

[xcaller_chainid_pairs]
mock_l1 = "mock_l2"
mock_l2 = "mock_l1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Some thoughts on config.

  • let's make it clear these are part of loadgen config
  • we use chain names, not chain ids.
  • do we have a reason for specifying which xcalls to to make xcalls from / to? if not, let's just do all of them. until we have a reason for more granular control
[loadgen.xcalls]
enabled = true

if we do have a need to specify, I think pairs are limitting. for ex, we could not loadtest xcalls on all streams by configuring pairs (at least, not as written). What about just specifying source and destination chains with a regex

[loadgen.xcall]
from = "mock_l1|mock_l2"
to = "*"

@@ -132,6 +132,12 @@ type Manifest struct {

// FeatureFlags defines the feature flags to enable.
FeatureFlags feature.Flags `toml:"feature_flags"`

// XCallerEnabled defines whether to enable the xcaller load generator for the monitor app.
XCallerEnabled bool `toml:"xcaller_enabled"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets follow the existing loadgen pattern, "Validator self-delegation" is simply enabled by presence of the key. Also, avoid complex configuration options in v1, simply hardcode the chain pairs in monitor. lessismore.

@@ -51,6 +53,21 @@ func (c Connector) Backend(chainID uint64) (*ethbackend.Backend, error) {
return ethbackend.NewBackend(chain.Name, chainID, chain.BlockPeriod, cl)
}

// GetPortal returns an OmniPortal binding for the portal at the provided address.
func (c Connector) GetPortal(chainID uint64, backend bind.ContractBackend) (*bindings.OmniPortal, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

xconnector isn't used in monitor, it is only used in scripts, lets keep it that way. please remove this.

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