Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi: add specialized rebroadcast handling for stake txs. #979

Merged
merged 1 commit into from
Jul 6, 2018

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Jan 22, 2018

resolves #975

server.go Outdated
@@ -179,6 +180,8 @@ type server struct {
txIndex *indexers.TxIndex
addrIndex *indexers.AddrIndex
existsAddrIndex *indexers.ExistsAddrIndex

pendingInvs map[wire.InvVect]interface{}
Copy link
Member

Choose a reason for hiding this comment

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

Please don't move this out of the rebroadcast handler. It's no longer concurrent safe moving it out here. You've introduced races by doing this.

blockmanager.go Outdated
@@ -2077,6 +2077,13 @@ func (b *blockManager) handleNotifyMsg(notification *blockchain.Notification) {
}

for _, stx := range block.STransactions()[0:] {
// Remove rebroadcast inventory for revocations
if txType := stake.DetermineTxType(stx.MsgTx()); txType == stake.TxTypeSSRtx {
Copy link
Member

Choose a reason for hiding this comment

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

This section is incorrect in at least 2 ways:

  • A block being connected does not invalidate a revocation. It's only invalidated if it was a revocation for a ticket which was previously invalid, but is now valid again
  • It's skipping the required mempool updates below due to the continue

blockmanager.go Outdated
@@ -2173,6 +2180,13 @@ func (b *blockManager) handleNotifyMsg(notification *blockchain.Notification) {
}

for _, tx := range block.STransactions()[0:] {
// Remove rebroadcast inventory for revocations
if txType := stake.DetermineTxType(tx.MsgTx()); txType == stake.TxTypeSSRtx {
Copy link
Member

Choose a reason for hiding this comment

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

Same here regarding this being incorrect. A block being disconnected does not necessarily invalidate a revocation.

@davecgh davecgh added this to the 1.3.0 milestone Apr 30, 2018
@dnldd dnldd force-pushed the rebroadcastdifftxtypes branch 7 times, most recently from 2ca40aa to df39b0a Compare July 6, 2018 00:46
@davecgh davecgh merged commit 9b08b58 into decred:master Jul 6, 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.

Add specialized rebroadcast handling for RPC server stake transactions
2 participants