Skip to content

Conversation

@aguycalled
Copy link
Member

In response to #399 , this PR introduces support for the -stakingaddress argument which sets a navcoin address where the staking rewards are accumulated.

Not compatible with cold staking.

@marcus290
Copy link
Contributor

@aguycalled function works for me but the wallet gui shows the wrong destination address for me:

screenshot from 2019-01-22 13-31-49

Also, seems like gettransaction rpc command will show the wrong address (I think the vin address):
screenshot from 2019-01-22 13-33-45

I think stop = true; // only one coinstake output in rpcwallet.cpp (line 1908) will cause some issues.

@@ -1,34 +1,47 @@
// Copyright (c) 2014 The Bitcoin developers
// Copyright (c) 2014-2018 The Bitcoin Core developers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to staking? Seems like it's been mixed in

txNew.vout[1].nValue = blockValue;
}

if (GetArg("-stakingaddress", "") != "" && !txNew.vout[txNew.vout.size()-1].scriptPubKey.IsColdStaking()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we validate a block do we check anything specific about a staking output?

@proletesseract
Copy link
Member

This is a great MVP but i would argue that we should add a little more functionality before releasing it into the main net.

As a user I can have multiple staking addresses which i might want to send their rewards to different reward addresses. I think we should make this value more of a map rather than a single value where i could specify "staking address:reward address" pairs. Or have an "all:reward address" which handles every staking address not otherwise specified in its own pair.

@matt-auckland matt-auckland added this to the 4.5.2 milestone Jan 31, 2019
alex v and others added 5 commits February 11, 2019 15:43
* fixes loop to only use the address from the last vout

* avoid cfund vout in coinstake
* fix list transaction rpc to show correct staking address

* avoid cfund vout in coinstake and fix amount in GetReceived()

* add test for -stakingaddress
Copy link
Contributor

@marcus290 marcus290 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manually checked, added python test and it includes the features suggested by @craigmacgregor

@matt-auckland
Copy link
Member

matt-auckland commented Mar 28, 2019

Tested and it appears to work. I'm not sure what the syntax is for staking to multiple addresses, so I haven't tested it. This could do with some documentation.

@aguycalled aguycalled merged commit 2fb7b47 into navcoin:master Mar 28, 2019
@aguycalled aguycalled deleted the address-staking branch March 28, 2019 21:37
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