Skip to content

Conversation

@luke-jr
Copy link
Member

@luke-jr luke-jr commented May 9, 2012

  • Transaction fees for children might help the parent transaction get confirmed
  • Transaction fees can boost the priority of transactions included

@luke-jr
Copy link
Member Author

luke-jr commented May 9, 2012

So far, I have done some minimal testing by analyzing the output of getmemorypool.

@luke-jr
Copy link
Member Author

luke-jr commented May 10, 2012

Eligius has found 2 valid blocks (and none invalid) with this code (plus #1246, for safety) so far, at its default settings. I am now testing with a huge weigh toward transaction fees, to increase the likelihood of dependent transactions paying for their parents (and therefore risking any potential child-before-parent problems).

@luke-jr
Copy link
Member Author

luke-jr commented May 10, 2012

Things I need to address:

  1. False "may be used uninitialized" warnings
  2. Config option names differ between actual code and -help
  3. Second-pass priorities should be printed with -printpriority
  4. Fee influence should be multiplied by total input value (input please: is this a good idea? should the total input be reduced by the fee amount first?)
  5. -printpriority shouldn't require fDebug (is this ok?)

@luke-jr
Copy link
Member Author

luke-jr commented May 11, 2012

FWIW, 10 total valid blocks found with this (most recent is …30E6C09A4FF45348A0EF0AA1A), zero issues caught by #1246, and zero invalid blocks logged by bitcoind.

@luke-jr
Copy link
Member Author

luke-jr commented May 12, 2012

Integrated my 5 "things to address".

@luke-jr
Copy link
Member Author

luke-jr commented May 19, 2012

Eligius has been running this from block 179513 (56 blocks found) and EclipseMC from 180573 (11 blocks), totalling 67 valid blocks with no problems found.

(EMC: 180788(1040); Eligius: …9DA7D49DC2539F9D299AF8E5A)

@luke-jr
Copy link
Member Author

luke-jr commented May 23, 2012

Also passes the unit tests I just wrote on #1246.

@luke-jr
Copy link
Member Author

luke-jr commented May 30, 2012

This seems to be be excessively slow, and possibly exploitable right now.

@sipa
Copy link
Member

sipa commented May 30, 2012

Define "this" ?

@luke-jr
Copy link
Member Author

luke-jr commented May 30, 2012

This pull request. Eligius is getting stuck in some huge dependency trees, it seems. Trying to figure it out, but don't have time just this second.

luke-jr added 5 commits June 12, 2012 19:39
… use %12"PRI64d" instead

Conflicts:

	src/walletdb.cpp
- Transaction fees for children might help the parent transaction get confirmed
- Transaction fees can boost the priority of transactions included
Conflicts:
	src/db.cpp
	src/init.cpp
	src/main.cpp
	src/main.h
@luke-jr
Copy link
Member Author

luke-jr commented Jun 12, 2012

Fixed the major performance hit from complex dependency trees. Still not as fast as the old algorithm, but not absurdly slow either. I'd say the ability to pay for "parent" transactions is worth it.

@luke-jr
Copy link
Member Author

luke-jr commented Jun 12, 2012

(Fix verified by about 10 days of testing on Eligius)

@sipa
Copy link
Member

sipa commented Jun 14, 2012

I certainly agree to the idea of this patch. I haven't checked the source yet though.

@gavinandresen
Copy link
Contributor

Need to be very careful not to accidentally introduce a potential DoS attack by arranging transactions that require N^2 time or space to figure out fees/priority.

@luke-jr
Copy link
Member Author

luke-jr commented Jun 19, 2012

@gavinandresen I think SatoshiDice tested that for me (the fix from ~16 days ago)

@gavinandresen
Copy link
Contributor

Could you write up a gist that explains what formula for priority you're using? And maybe talk about how it will handle transactions from old clients (who calculate priority the old way) ?

@luke-jr
Copy link
Member Author

luke-jr commented Jul 13, 2012

  • DepthWeight = user configurable (default: 1.0)
  • FeeWeight = user configurable (default: 1.0)
  • DepthScore = sum(sum(value * depth) for each input)
  • FeeScore = sum(value for each input) * fee
  • WeighedScore = (DepthScore * DepthWeight) + (FeeScore * FeeWeight)
  • EffectiveSize = datasize + sum(datasize for each (transaction this depends on that is not yet in a block))
  • Priority = WeighedScore / EffectiveSize

Old clients don't take priority into consideration at all right now AFAIK, nor does this code (or the code it's replacing) completely exclude transactions based on their priority.

@gavinandresen
Copy link
Contributor

NACK, the priority formula would make it easy for somebody with (say) 100,000 BTC to pay tiny fees and get priority over higher-fee-paying transactions. That's a bad incentive, it would encourage people to do dangerous things like keep more funds in their 'hot' wallet or add extra inputs to their transactions (and just have bigger change outputs) to get higher priority.

@luke-jr
Copy link
Member Author

luke-jr commented Jul 13, 2012

So, should I take the input value out of FeeScore?

@jgarzik
Copy link
Contributor

jgarzik commented Aug 1, 2012

Sadly, in addition to being NAK'd this conflicts heavily with @gavinandresen 's work redoing CreateNewBlock() You should work with @gavinandresen to coordinate changes in that area, before posting another pull request relating to CreateNewBlock() + TX selection/fees.

suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017
…1240)

* Reject getblocktemplate request until masternode sync is finished

* Pending internal miner until masternode sync is finished
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 22, 2019
* Do no relay double spends to SPV clients.

Relaying double spends to SPV clients can be easily attacked.  An attacker
could intially send tx1 to their own wallet. Then send a double spend (tx2) to
another merchant's  SPV wallet. The merchant will never see tx1 but will
see and receive tx2, however, tx2 will not get mined since the miners node
will see tx2 but not add it to their mempool. Later, tx1 will get mined
leaving the merchant with an invalid/conflicting tx2 and no payment will
be confirmed.

* Only skip sending Respends to peers that have given us a bloom filter which
is not empty.
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request May 6, 2020
…xRecord updateStatus.

0057548 [Model][Wallet][Performance] TxRecord updateStatus not accepted stake status fix + performance improvements. (furszy)

Pull request description:

  Several improvements on the `updateStatus` method solving the not accepted stake type not set bug, plus performance improvements.

  * The transaction record status update was not taking into account the negative depth calculation, neither if it was conflicted, for not accepted coin stakes

  * GetDepth method was called in three different ways. 1) GetDepthAndMempool, 2) IsTrusted, 3) GetBlocksToMaturity. This PR unifies every call into a single call to IsTrusted(&depth, &fConflicted).

  * Several calls to chainActive->Height unified into a single one in record statusUpdate.

ACKs for top commit:
  random-zebra:
    utACK 0057548
  Mrs-X:
    utACK PIVX-Project@0057548

Tree-SHA512: 9517b1bd2bb782bd459ef94972db04a0408429e5f38009f178c915ae24e8240f792c30e884f809a91f605fcc2dae9aa6fef3dd8a74d1e77c973014f1b346c683
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants