please no tag yet

K.

I haven't started on the release merges yet.

Looking over 7102 before I start in on the branches.

Clear to get all the merges into the release branch, even if not tagged yet selsta?

should be good

The merging shall continue.

:D thanks

Merges done

yay!

wiat Snipa don't do anything

10k commits.

Hahaha.

Preserve this moment in history?

ok u can touch things again

moneromooo selsta I see a lot of rushed PRs yesterday. While they seem rather simple, more testing is needed instead of just tagging .7. Eventually a mistake can creep in the code that will fracture the p2p network by nodes banning each other in some conditions. This is what attacker wants.

We did not tag yet.

I'm just saying don't tag right after all PRs are merged

yep

A few people (like 10 at least) need to run them and see if they ban a lot of innocent peers or not

Mooo and I have been running them since yesterday. Hope others try them too.

But I doubt 10 people will try them. Even with large releases we don't have 10 testers.

Seeing monero-project/monero #7123 is already worrying me

I can test this time :)

mooo found 7123 with testing :D

7123 does not seem correct to me

it breaks the detection we wanted in the first place

but it has many false positives

it did not in my testing, maybe for others

all IPs that were banned after running for 12 hours were out of my block list

but the patch has a reason most likely, so more work is needed there

"We can actually request a chain that's further away from what we have as we buffer more and more"

So maybe syncing peers can be blocked by this?

has been running the master-ebdc617 for hours, what logging level needed or something settings for test?

do you see IPs getting banned that are not in gui.xmr.pm/files/block.txt ?

just now

2020-12-11 09:09:43.329 I Host 87.98.224.123 blocked.

2020-12-11 09:12:07.180 I Host 145.239.118.5 blocked.

I Host 51.68.222.162 blocked.

these are fine

All OVH

sech1: it is likely that the issue only becomes visible with --log-level 1

because innocent peers would only get kicked once / twice

still something we have to fix

2020-12-11 09:09:24.588 E Exception at [core::handle_incoming_txs()], what=ge_frombytes_vartime failed at 380

this is unrelated

Some peers are banned because they're too slow. If that's the reason, the comms get reset, and in turn this gets them kicked by the "receiving message out of sequence" system. That's a bit annoying, but acceptable to me.

That'll be made more lenient sometime later, needs something else first.

Are you talking about 7123?

No.

So what was wrong about 7123? I asked about that line of code yesterday and you said it was ok :D

with 7123 the +2 detection stops working

Oh really.

OK, I'll look at it soon.

could allowing `--ban-list url` lead to security issues?

Some people find it difficult to download the file and specify the patch correctly. This would also allow that users stay up to date.

the path*

this can be solved by a simple bash script (run wget, run monerod) for startup

no need to add it in monerod

Yes, but Windows users (who have to most issues) don't know how to use a bash script.

Not sure if bash even runs on Windows.

not all Windows users obviously, those that have issues

It's unrelated, it'd happen also before yesterday's changes.

New patch pushed.

together with 7123?

No, new patch.

7127

right, but should I test together with 7123?

7123 + 7128 looks good again, +2 attack getting stopped

Yes, and all others you intend to use for the release.

ok

I have been running release-v0.17 + 7123 + 7128, looks good so far, will let it run a bit to check for innocent bans

7129 increases the number of out connections when syncing, some people reported a substantial sync time decrease with more out peers.

It'd be interesting to see if this patch also shows the improvement, and whether it makes things worse for others.

It doesn't seem to help me, for example.

I had the idea to increase out_peers until the last checkpoint, but doing it in sync mode generally should also work.

I think it does hinder if your connection is maxed out already with 12.

ive got master up for 10 hrs now

.merge+ 6495 7123 7124 7127 7128

Added

Whoever wants to review, see 7131. I'll be using that later, so it'd be nice to have a good portion of nodes already setup to include this data, though it is noop now.

Basically, check PoW for the first block before considering a claimed chain.

(and the rest isn't ready, though it works as a PoC)

7132 sounds like it defeats the purpose of target_height

It's also set when getting the blocks.

Getting the first block in the series, I mean.

you just closed these anyway, so, didn't work out as expected?

Apparently. Worked for me, but not selsta. And tbh I can't be bothered debugging that one since this is just an addon anyway.

It should get closed once we have the PoW check. Which will be... probably at next fork, to avoid disrupting.

selsta, Ummm we still have an issue?... i.imgur.com/6Z9pcWJ.png

check which IPs in sync_info show +2 target height and post them here

I closed the daemon and reopened it

it's now at 100%

if it does it again I will post for you.

Kronovestan: do you have the ban list applied?

no

We might need one or two more releases to resolve this issue.

10-4

In the meantime use the ban list.

I will reapply ban list.