still showing target height 2228507, even though current height across many peers is 2228824

Hi guys!I've been using the xmr rpc node for a while, but just now i faced a minor issue, but it's really important to solve. I don't understand how I can get info which node version I am currently using. I found rpc get_version method, but with this method I get the rpc wallet version, but I need to get the node version. With rpc get_version

method I get the installed version of rpc, but I want the version of the node itself that I installed, i.e. as on github monero-project/monero/releases/tag/v0.17.1.1or as in the monero site in Downloads part (Current Version: v0.17.1.4 - Oxygen Orion)For example, I am using node v0.17.1.1 and rpc wallet version v3.1. And I need to get the node

version i.e. v0.17.1.1 .Is it possible?a piece of advice from you would be really appreciated!

Why is it important to solve it ?

hyc: are you in sync mode ?

moneromooo: node is all caught up

Then you are not in sync mode.

in which case, what? target height doesn't mean anything?

Right.

this log message is about being unable to relay txns. if the target height only means something in sync mode, then it's irrelevant here

because a syncing node isn't supposed to be relaying txns anyway

which means this commit 6352090e6d7f3c639ab9eec23a62304e785cb91b is wrong

xiphon: maybe you should have used the actual local blockchain height, instead of target height in that patch

hyc: example: your node connected to 2 peers. Your height is 1, peer A height is 2 and peer B height is 3

target height should be 3

local height is 1

if you use local height, you will send a tx to peer A as well though it is not synced

would actually tweak it and use std::max(local_height. target_height)

xiphon: as mooo just said, target height is meaningless when your node is syncing. and when your node is syncing, it is not relaying txns.

Does this allow some asshole to claim height 1e9 and prevent all txes being sent to *others* ?

yeah that seems likely too.

this patch is just wrong.

nope, it is not

"Does this allow some asshole to claim height 1e9 and prevent all txes being sent to *others* ?"

you should drop the connection

if it doesnt provide the blocks

It takes a few minutes IIRC.

To be nice to honest slow peers.

Or maybe do this ignoring only if the node claiming H > local has sent at least one valid block along the way,

xiphon: you're testing target height when target height is meaningless. so regardless of anything else, the patch is wrong.

it shouldn't be meaningless

the peers provide target height

if it doesn't work, then it is a bug

the patch is correct

just fix the target height

instead you want to revert the correct patch

and leave the target height but there

no, I want to fix the patch. target height is the wrong value

why do you think so?

WTF man pay attention

As long as it's not vulnerable to DoSing relay to other peers, I'd be happy with the result.

i'm pretty confident in what i'm saying

2020-11-12 14:40:49.861 W No suitable outbound tor connections at height 2228897 - currently unable to send transaction(s) to this zone

status

Height: 2229060/2229060 (100.0%) on mainnet, not mining, net hash 1.72 GH/s, v14, 12(out)+21(in) connections, uptime 1d 0h 9m 48s

the value has no relation to reality

i see no problem target height being the max of the peers' heights

my node has been logging this message with target height for hours

^ "<xiphon> would actually tweak it and use std::max(local_height. target_height)"

in all cases, the target height is far out of date wrt to actual chain

and even if we tweak as you suggest, using max still means a lying node can shutdown all txn relaying.

nope

your should drop the lying node

if it claims the 1e9 height

... we're talking in circles

but doesn't provide the blocks

Just fix it I think.

Just fix which part?

The dependency on the target value when there is no target.

so it should just use local height

yep, and eventually send txes to desynced peers

The DoS prevention can then come later, if there is such a DoS vector, xiphon says not but it'd be nice to make sure.

and then break the dandelion

xiphon: you're still not paying attention. this section of code only runs when the node is sync'd.

unless you're saying that with dandelion, nodes will relay txs even when they're not syncd?

hm, nope

i can send txes being at height 1

vtnerd: btw, I added a scoring system for peers recently. Do you think selecting dandelion targets based on score would be a good idea ?

ie, give much lower probability to low score peers.

mean "this section of code only runs when the node is sync'd" -> i can send txes being at height 1

Or would that remove too much entropy from picks...

in that case, the value should be local height when node is sync'd, target height when not sync'd.

but usually a node will not accept txs while syncing. the rpc will return BUSY

so I dunno what you're doing.

^ ./monerod --no-sync

the RPC will allow sending txes to P2P even if you ain't synced

gawd, why do we have an option like that. means it can't validate the txn before relaying.

when a user don't want to sync all the blockchain just to send a tx

It's the third party node sickness that's propagating. IIRC it's used for the "find me a public node via P2P but don't sync" mode.

actually txes validation is other peers' business

It's kinda both good and bad at once :/

sounds like that patch was written with only a non-syncing node in mind

(the current target height patch that is)

it is still wrong for the default case of a normal node.

"in that case, the value should be local height when node is sync'd, target height when not sync'd"

that's std::max(local_height. target_height)

either i wounder how would one know it is synced or not without comparing local height and target height

s/wounder/wonder/

stop wondering and read the damn code

is_synchronized() is already there

it is damn "if (m_core.get_current_blockchain_height() >= m_core.get_target_blockchain_height())"

if (context.m_remote_blockchain_height >= m_core.get_target_blockchain_height())

{

if (m_core.get_current_blockchain_height() >= m_core.get_target_blockchain_height())

{

MGINFO_GREEN("SYNCHRONIZED OK");

on_connection_synchronized();

so again, target is not meaningful on its own

checking local height and remote height

target height is max(all peers' m_remote_blockchain_height)

and m_remote_blockchain_heigh is just the current peer's height

no, target_height is not *only* that. it gets reset when a connection closes

in void t_cryptonote_protocol_handler<t_core>::on_connection_close(cryptonote_connection_context &context)

in particular, it can get set down to zero if there are no connections in the current zone

which is arguably the wrong thing to do if there are still other zones with live connections

sending is different than relaying

I guess max(local height, target height) will be enough.

... right? in this discussion "<xiphon> the RPC will allow sending txes to P2P even if you ain't synced" . dunno if its important to distinguish tx initiation and relay

anyway, the current code in repo is wrong.

"anyway, the current code in repo is wrong." -> will send a PR soon

"<gingeropolous> sending is different than relaying" -> i would say that tx validation on sending is kinda optional (you are actually double-checking the tx you have just generated)

but when you relay someone's else tx, you must validate it to prevent spamming

Do public RPC nodes verify in the RPC ? If not, they can be uesd to relay spam.

Though a solution is enabling pay-for-rpc.

"Do public RPC nodes verify in the RPC" -> they do

still something suspicious about this. if target height is max(all peers remote height)

then that if statement makes no sense:

if (context.m_remote_blockchain_height > m_core.get_target_blockchain_height())

also - since you've already established that a non-sync'd node can send txs, but this code prevents relaying txs thru a non-sync'd node,

then you know for certain that if you receive a tx from a non-sync'd node, it was the originator

kinda defeats the purpose of running a node, doesn't it?

"if you receive a tx from a non-sync'd node, it was the originator" -> yep, though since the sending node doesn't fluff a tx, only one peer will know that. Anyway that's not good. It is primarily used in GUI Simple Mode and it warns a user on privacy implications of using Simple Mode (i.e. --no-sync + auto remote node)

"a non-sync'd node can send txs" -> was added cause sending txes through auto remote nodes wasn't that reliable

and there were a lot of users' reports on missing/failed/non-confirmed txes

this might be a dumb Q but if the node isn't synced how does it even know an output is spendable? or are we just counting on synced nodes to sort that out

the latter

seems like missing/failed txs was probably all due to the asshole nodes

I don't see how sending tx from a non-sync node is any more reliable than sending directly to a remote node

We do both now

thus.. more reliable

so we send the same tx to remote node and to P2P

"it was the originator" -> i actually don't like that. But the only way to fix it is to revert it back and don't send txes from a non-synced node

is there any reason why a non-synced node can't query the current blockheight from node A, then broadcast transaction to node B

in which case the non-synced node would appear synced to node B

GUI Simple Mode warning -> i.postimg.cc/CK9Ssmvy/simple-mode.png

jwinterm: a node reports its height to each of its peers

so the only way would be to "mirror" the peer's height

what is a hack and is a no-go at a glance

that's basically what the asshole nodes were doing

ic

moneromooo : I wouldn't recommend a weighted dandelion stem selection algo right now

a normal node will temporarily drop in score when "testing" you, but spy node could almost remain highest

Good point.

so I dunno, which is frustrating because its an alternative way to wreck small bit of havok

throw out highest score?

I am currently syncing a new node from scratch and I would have that bad node block height mirroring issue on a daily basis causing my sync to stop, so I would have to reboot monerod every day to get the sync going again. After upgrading to the latest version of monero and adding the --ban-list flag I have had ZERO issues of my sync freezing. So thanks a bunch for finding this issue and mitigating it! I am on

track to be at the tip soon! 🙌