Skip to content

Conversation

@sidhujag
Copy link

Associated with issue #1734 in that txindex was being forced upon nodes to ensure that masternode collateral was checked properly. Pruned nodes would not work without txindex because they wouldn't have the collateral transaction history to be able to verify old masternodes.

However by using addressindex which is pruning compatible and a lot lighter than txindex we can verify masternode collateral without txindex (must set addressindex instead).

Also not that dual calls of GetTransaction in CheckOutpoint can be easily avoided by returning the height from IsInputAssociatedWithPubkey since addressindex returns height when UTXO was created.

Other calls to GetTransaction stay as is, one is in CGovernanceObject::IsCollateralValid where it will do a slow lookup but also assume that collateral objects are fairly new and new nodes will generally keep a months worth of data around to be able to verify valid proposals.

The other call is in CInstantSend::ResolveConflicts and again there is an assumption that pruning nodes would atleast have enough data around to lock and verify an instantx.

Associated with issue #1734 in that txindex was being forced upon nodes to ensure that masternode collateral was checked properly. Pruned nodes would not work without txindex because they wouldn't have the collateral transaction history to be able to verify old masternodes.

However by using addressindex which is pruning compatible and a lot lighter than txindex we can verify masternode collateral without txindex (must set addressindex instead).

Also not that dual calls of GetTransaction in CheckOutpoint can be easily avoided by returning the height from IsInputAssociatedWithPubkey since addressindex returns height when UTXO was created.

Other calls to GetTransaction stay as is, one is in CGovernanceObject::IsCollateralValid where it will do a slow lookup but also assume that collateral objects are fairly new and new nodes will generally keep a months worth of data around to be able to verify valid proposals.

The other call is in CInstantSend::ResolveConflicts and again there is an assumption that pruning nodes would atleast have enough data around to lock and verify an instantx.
@nmarley
Copy link

nmarley commented Nov 27, 2017

As far as I can remember, the MN requirement isn't an address, but a 1000-Dash UTXO. What happens in the case where multiple 1000-Dash UTXOs (potential masternodes) are associated with the same Dash address?

I know this used to be possible, and some were running MNs this way. Not sure if valid any longer, but if so, then this would break that, right?

@sidhujag
Copy link
Author

@nmarley It would loop and find the correct TXID based on address index, where do you see it breaking exactly?

Instead of looking up TXID then check script(address), it looks up based on script(address) and then checks TXID for a match.

@UdjinM6
Copy link

UdjinM6 commented Nov 28, 2017

I guess it makes sense to allow pruning on non-masternode nodes if possible. Such nodes can simply rely on UTXO db for verifying MN collateral (collateral can't be spent for an active MN). I'm not sure about other places like for IS, but maybe there is a workaround too.

However, I don't think that allowing MNs to be pruned nodes is a good idea in general - they serve as a backbone of the network and they are paid to do this job, so I would actually do the opposite thing and add some proof-of-blockchain as a requirement for MNs instead.

@sidhujag
Copy link
Author

sidhujag commented Nov 28, 2017

@UdjinM6 Well the costs to run the masternodes then rise over time quite a bit. A bigger thing for me is the requirement of txindex because if you take that away then it becomes slow to do lookups in GetTransaction and addressindex fixes that with a smaller index that does what we need it to do at the same time as allowing pruning to do its thing. The GetTransaction calls in IS/GO are likely adequate since generally they are new pieces of data and not affected by pruning. However the GetTransaction will be a slow lookup in those functions so since you are probably more familiar with how often these will get called you would have a better idea in terms of performance tradeoffs versus reduced storage costs that addressindex gives over txindex.

@UdjinM6
Copy link

UdjinM6 commented Nov 28, 2017

The cost to run masternodes will rise, there is no doubt but each one is making $4k+ per month (at least the last time I checked), so you can run it on a pretty decent hardware already, shouldn't be an issue. But if running a masternode becomes even more costly that its returns - it's not a big deal either because all masternodes share the same subsidy i.e. the more masternodes drop from the network, the higher share for each of those still online. It's a self-balancing market, should be ok imo.

Even though I agree that IS operates with the most recent data and thus GetTransaction should work most of the time without txindex too, it will fail if all outputs are already spent in descendant txes i.e. it's neither in mempool nor in utxo. That would be ok if you would be able to prove that tx was indeed included in some block but for this case you can't do this without txindex, it could be that tx was simply dropped from the mempool for some reason. You can probably add some additional logic (and caches) to check this edge case but I'm not sure it would worth it because... see below.

As for GO, dropping txindex won't work here at all imo since gov objects can be valid for many months or even a year, there is no limit atm. Moreover we need to find actual collateral txes because we have to check them to make sure objects are valid but a) these GO collaterals are txes with one OP_RETURN (and data output is never in utxo) and b) other outputs are not required to be unspent either (unlike for MN collaterals), so utxo is not going to help here.

@sidhujag
Copy link
Author

Yea I saw that collateral check. Its unfortunate as that is whats stopping the requirement of txindex from being dropped. Is there not a way to derive the info required to check the collateral through the available info without txindex assuming pruning enabled?

@UdjinM6
Copy link

UdjinM6 commented Nov 30, 2017

I don't think so - block is essentially just a set of txes + a header, so there is obviously no place to get txes from, if you prune them...

@sidhujag
Copy link
Author

sidhujag commented Dec 1, 2017

Can we not assume that if your chain isn't syncing it is because txindex is off and or your pruning too much? please increase the size your willing to keep on your HD and try again? obviously those that prune down to say a few hundred megs might hit an issue but in general governance objects lifespan isn't incremental is it? They do go away do they not?

@UdjinM6
Copy link

UdjinM6 commented Dec 1, 2017

Well, an easier way would be to let pruned nodes just skip validation of govobject collateral - they don't really add any meaningful value to the network anyway, so the fact that they can be banned for relaying invalid govobject sometimes is not such a big deal (for the network).

@sidhujag
Copy link
Author

sidhujag commented Dec 2, 2017

If they skip validation does that stop them from being banned for relaying empty gov objects or does it not relay them because it doesnt have them I guess? So I guess that makes sense but strength of consensus becomes a variable of pruning so it should be clear.

@UdjinM6
Copy link

UdjinM6 commented Dec 3, 2017

If it can't validate it won't relay. And yes, this makes things a bit more fragile. That's why I said that proof-of-blockchain should be a requirement for MNs and pruning should not be an option for them - they not only serve blockchain and validate and relay govobjects, but they also actually use govobjects to construct other govobjects (e.g. proposals->trigger) and to vote (MNs vote for triggers). If you have at least one connection to any MN you should be pretty sure that you'll be able to get both full blockchain and all govobjects.

@codablock
Copy link

Is it really a good idea to have (non-masternodes) nodes that don't relay something? I think we always kind of assume that if you relay something to one node, it will eventually be relayed to other nodes as well.

Also, IMHO, if people run a full node, pruned or not, it should validate everything. Otherwise it's not a full node. It doesn't feel right to add exceptions to this.

@nmarley
Copy link

nmarley commented Apr 10, 2018

This should no longer be an issue now that lite mode has been fixed in #2014. I think the consensus is we want to keep MNs as full nodes and not enable pruning on MNs. This is especially important as Evolution will require much more from masternodes than has been previously.

@nmarley nmarley closed this Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants