feat: remove effective_canister_id from fn fetch_api_boundary_nodes() signature

[nikolay-komarevskiy]

Description

Following the spec, it would make sense to split fn fetch_api_boundary_nodes(effective_canister_id) into two functions:

pub async fn fetch_api_boundary_nodes_by_canister_id(canister_id)
pub async fn fetch_api_boundary_nodes_by_subnet_id(subnet_id)

How Has This Been Tested?

1. Testing against mainnent is not yet possible, this MR needs to be released.
2. Functions were tested in system-tests with respective canister_id/subnet_id.

Checklist:

• The title of this PR complies with Conventional Commits.
• I have edited the CHANGELOG accordingly.
• I have made corresponding changes to the documentation.

[mraszyk]

I think the new function should complement, not replace the old one. There are use cases when you might want to fetch the API boundary nodes based on a canister you want to talk to.

[nikolay-komarevskiy]

I think the new function should complement, not replace the old one. There are use cases when you might want to fetch the API boundary nodes based on a canister you want to talk to.

fn fetch_api_boundary_nodes_via(effective_canister_id)
fn fetch_api_boundary_nodes()

Like this?

[mraszyk]

I'd go for fetch_api_boundary_nodes_via -> fetch_api_boundary_nodes_for_canister but that's the idea, indeed.

[nikolay-komarevskiy]

fetch_api_boundary_nodes

@mraszyk Once the support for getting API Boundary Nodes via subnet/subnet_id/read_state is done, we could use it in the fn fetch_api_boundary_nodes(). Or would it make sense to introduce yet another function fn fetch_api_boundary_nodes_via_subnet()?

[chenyan-dfinity]

This default canister id probably only works when talking with the mainnet. For local replica or testnet, r7inp-6aaaa-aaaaa-aaabq-cai may not exist?

[nikolay-komarevskiy]

This default canister id probably only works when talking with the mainnet. For local replica or testnet, r7inp-6aaaa-aaaaa-aaabq-cai may not exist?

Correct. To rectify this we could give a more precise name to the function: fn fetch_api_boundary_nodes_mainnet()

[nikolay-komarevskiy]

This default canister id probably only works when talking with the mainnet. For local replica or testnet, r7inp-6aaaa-aaaaa-aaabq-cai may not exist?

Correct. To rectify this we could give a more precise name to the function: fn fetch_api_boundary_nodes_mainnet()

What do You think @chenyan-dfinity and @adamspofford-dfinity?

[chenyan-dfinity]

Yeah, that's a more precise name, and we don't need to rename the original function then.

[nikolay-komarevskiy]

Yeah, that's a more precise name, and we don't need to rename the original function then.

In fact as @mraszyk has implemented api_boundary_nodes for subnet read_state, we could have these three function flavours:

 1. pub async fn fetch_api_boundary_nodes_by_subnet_id(subnet_id: Principal)
 2. pub async fn fetch_api_boundary_nodes_mainnet() // uses hard-coded root subnet_id internally
 3. pub async fn fetch_api_boundary_nodes_by_canister_id(effective_canister_id: Principal)

But it feels that the 3rd one is not necessary, I suggest to have 1 and 2.

[chenyan-dfinity]

I don't know how boundary node works, so I don't know what's the best API here. I think it depends on the following questions:

1. Do boundary nodes exist only in the mainnet? If so, it makes sense to have a hard-coded subnet/canister id.
2. With different subnet/canister id, do we expect to get a different list of boundary nodes? If so, we will need both 1 and 3. Currently on the mainnet, it seems the list is always empty?

[nikolay-komarevskiy]

I don't know how boundary node works, so I don't know what's the best API here. I think it depends on the following questions:

1. Do boundary nodes exist only in the mainnet? If so, it makes sense to have a hard-coded subnet/canister id.
2. With different subnet/canister id, do we expect to get a different list of boundary nodes? If so, we will need both 1 and 3. Currently on the mainnet, it seems the list is always empty?

1. API BNs will exist in three environments:

• Mainnet (now it's an empty array to be populated soon)
• system-tests
• Dynamic testnets

2. Provided the responding nodes have identical state the response for both subnet_id/canister_id should be identical.

I updated the current PR with a suggestion to have just two functions, which cover all our needs, see description.