chore(lib): validate inputs before locking

[chmllr]

The PR #2140 introduced a locking around a cache. Some parts of the critical section seem unrelated to the locking and can be done before without changing the read semantics (because this check happened after the cache read before).

issue: none

[chmllr]

@kevinhalliday I have another question to the original PR: how critical are lock contentions on GetAddresses? The original PR uses a simple mutex, while we could also use an RW-lock and separate cache reads and cache updates so that at least concurrent reads (which will be the most frequent accesses) are not blocking each other.

[corverroos]

@chmllr RWMutex should only be used for performance optimisation, I would not worry about that in that case. Performance is a non-issue. Lets keep things simple.

I would even argue that time spent on this PR by multiple people isn't worth the effort. The value it adds is insignificant and it feels like we are making the code harder to read.

A simple method with lock-defer-unlock as first line is common and standard and doesn't need much thinking. As soon as you move the lock around, readers of the code need to think why is this done here and not there, etc.

I wouldn't worry about these type of optimisations.

[chmllr]

@chmllr RWMutex should only be used for performance optimisation, I would not worry about that in that case. Performance is a non-issue. Lets keep things simple.

I would even argue that time spent on this PR by multiple people isn't worth the effort. The value it adds is insignificant and it feels like we are making the code harder to read.

A simple method with lock-defer-unlock as first line is common and standard and doesn't need much thinking. As soon as you move the lock around, readers of the code need to think why is this done here and not there, etc.

I wouldn't worry about these type of optimisations.

Thanks Corver! Fully agree with optimizing for simplicity first, that's why I asked how critical this contention is. Because the fact that we have a cache means we do have a lot of reads. So if they all wait for each other now, might not be what we want and make the whole situation worse than without any cache in the first place... But here I fully trust your intuition 👍