neo
neo copied to clipboard
neo-project
Remove unnecessary default seedlist
To be honest, i have no idea what the point of these settings since we already have everything in the config.json.
I agree with OP to remove this behaviour. It is counter intuitive and had me scratching my head multiple times in the past. If SeedList means the list of nodes to seed from, then an empty list should mean "do not connect to any seed". Having to input some non-existent ip address just so it stops connecting to MainNet feel bad.
The whole hardcoding default config seems like some attempt to avoid null pointer exceptions due to invalid configs (iirc from the past). It shouldn't be that hard to check the config against a JSON schema and shutdown in case of an invalid config instead of doing this default thing.
But not worked as this behaviour, see: https://github.com/neo-project/neo/issues/2772#issuecomment-1162706000
And we already have them in config.json, why someone have to remove seedlist[]? Can't see any scene like this. But make seedlist[] empty is an ordinary behaviour in single consensus privatenet, it shouldn't connect mainnet seeds.
But make
seedlist[]empty is an ordinary behaviour in single consensus privatenet, it shouldn't connect mainnet seeds.
This ☝️
@shargon can you explain what are you're trying to say with this comment? We can read that the behaviour is "by design" according to the dotnet team, but I don't think the current behaviour in NEO core is the intention by the NEO team. At least the way I read Erik's comment is "when the whole seedlist key is missing from the config it should take the default seed list, "but" an empty array should give an empty seedlist". Empty seedlist to me means, do not connect to any seed nodes and that's not what it is doing as shown in my screen recording
I think it's necessary to make default seedlist empty, please move on. @erikzhang @shargon
What's the sense of remove the seedlist but not the StandbyCommittee? I think that we should remove all and ensure that it's configured by the json
What's the sense of remove the seedlist but not the
StandbyCommittee? I think that we should remove all and ensure that it's configured by the json
Actually I don't care about any other parameters because in no sense I need to make it empty on my config.json for privatenet, but seedlist has this sense. When I start a single consensus and a seed node or you call it operation node/debug node/terminal node, I input command, deploy contract on it and it doesn't need to connect any peers. However, it connects to mainnet seeds finally.
@ixje Maybe we can temporarily set seedlist like "SeedList": [""] to avoid this issue.
@ixje Maybe we can temporarily set seedlist like
"SeedList": [""]to avoid this issue.
That's a workaround similar to using a non-existent ip address, that is not a solution in my opinion. SeedList: [] meaning do not connect to anything is what I call developer friendly and I thought that's what NEO is trying to be.
@ixje Maybe we can temporarily set seedlist like
"SeedList": [""]to avoid this issue.That's a workaround similar to using a non-existent ip address, that is not a solution in my opinion.
SeedList: []meaning do not connect to anything is what I call developer friendly and I thought that's what NEO is trying to be.
I think we need to restrict that in Neo.Json to re-deserialize settings for all empty array to keep all in the same behaviour.
Let's handle this one for 3.7.0, either we fix it or not. It's a relatively simple thing, but some people find the default behavior annoying. And we have this PR for a long long time without any decision which is bad.
My take on it is that it should be merged, please vote :+1: or :-1: below and we either merge it (well, it needs a rebase now) or close it next week. @AnnaShaleva @shargon @vncoelho @superboyiii
I agree to merge this PR, but at the same time I agree with https://github.com/neo-project/neo/pull/2773#issuecomment-1173634283. If we're removing the default SeedList (mainnet's one) and suppose that the default configuration can be used for any network (testnet, privnet), then other mainnet-related fields also should be removed (mainnet standby committee, magic, etc.). Otherwise we have kind of "mixed" configuration that includes both mainnet-related things and truly "default" things.
But it's a nitpicking and not critical.
Are we really asking somebody to update his/her PR after almost 1.5 years? Looking at the persons profile shows no GitHub activity since creating this PR. I say just merge this and deal with Anna's suggestions in a new PR.
Closed by inactivity and moved to https://github.com/neo-project/neo/pull/2980
Are we really asking somebody to update his/her PR after almost 1.5 years
You never know. At least it was important for me to try updating/merging the PR from the original author. If it can't be done, OK, no problem, but we have tried.
I'm always there, the shadow 👻 . Really good to see this moving forward. Neo is awesome! @roman-khimov @ixje @shargon @vncoelho
laugh one's ass off
You never know when I will be back. May the force be with you. 😏
I'm always there, the shadow 👻 . Really good to see this moving forward. Neo is awesome! @roman-khimov @ixje @shargon @vncoelho
@satoshichou please check https://github.com/neo-project/neo/pull/2980
I'm always there, the shadow 👻 . Really good to see this moving forward. Neo is awesome! @roman-khimov @ixje @shargon @vncoelho
@satoshichou please check #2980
Yeah, already saw this. Good moving.
@satoshichou feel free to open again the pull request!
Yours is enough!