Bootnodes addresses review

[lrubasze]

Is your feature request related to a problem? Please describe.

Observed that:

1. if bootnode_addresses are set with with_bootnodes_addresses() there are no address replacements (introduced here)
   eg. {{ZOMBIE:alice:multiAddress}} is not translated into proper address
   Similar replacements are performed for args

2. bootnodes_addresses set for ParachainConfigBuilder are not taken into account eventually.
   In add_parachain() method we set bootnodes_addr = &vec![] (see this), despite input para_config could hold some values.

Describe the solution you'd like

1. Implement replacements
2. There is an option to set bootnodes_addresses at node/collator level so maybe it would make sense to get rid of with_bootnodes_addresses() for ParachainConfigBuilder?

Describe alternatives you've considered

No response

Additional context

No response

[pepoviola]

Hey @lrubasze, let's touch base later about this one.
Thx!

[lrubasze]

1. if bootnode_addresses are set with with_bootnodes_addresses() there are no address replacements (introduced implement context replacement #327)
   eg. {{ZOMBIE:alice:multiAddress}} is not translated into proper address
   Similar replacements are performed for args

Finally figured out what I meant here 😄

• If you configure node and specify bootnodes via args, eg.

node.with_args(("--bootnodes", "{{ZOMBIE:bob:multiaddr}}").into())

then bob's address is replaced with multiaddress.

• If you try to configure bootnodes with with_bootnodes_addresses(), eg.

node.with_bootnodes_addresses(vec!["{{ZOMBIE:bob:multiaddr}}"])

then it does not work.

Some real use case example withing polkadot-sdk: https://github.com/paritytech/polkadot-sdk/blob/277a26585b3d7e38668d5068d2f10ef39051f5d6/cumulus/zombienet/zombienet-sdk/tests/zombie_ci/pov_recovery.rs#L231

cc: @mchristou

[pepoviola]

hm, thanks @lrubasze. This could be tricky since we make the replacements in args when we generate the final command to execute. I will double check the code to find a design that allow use to use replacements in other methods too.

Thx!

[pepoviola]

blocked: Needs investigation (@pepoviola)

[lrubasze]

Thanks!
Additionally (as @mchristou mentioned in 1:1 conversation) with_bootnodes_addresses accepts parameters, which are convertible into multiaddress, which is not the case before replacement.

Not sure, maybe we can just live with it as it is now? We could just make sure description clearly states that replacements are not applicable here.

@pepoviola @mchristou WDYT?

[pepoviola]

Thanks for the info @lrubasze. Yes, maybe we can document the fn to clearly state that this should be raw multiaddress and if you need replacements you can use the args. We can even call the method with_raw_bootnodes_addresses to make it more explicit.
wdyt?

[lrubasze]

It makes perfect sense for me 👍

[pepoviola]

ping @mchristou can you add some ///docs to make clear that replacements are only run with the args and not in other methods like with_bootnodes_addresses also, rename with_bootnodes_addresses -> with_raw_bootnodes_addresses with a deprecation notice.

Thx!