Serious bug with manual covers

[pmconrad]

See https://bitsharestalk.org/index.php?topic=14271.msg185630#msg185630

Here's my theory. The problem is that the payout of the collateral is not handled in the same place as the actual filling of the cover.

The manual cover transaction is created by transaction_builder::submit_cover. That adds a cover operation to the transaction and deducts the cover amount from the transaction balance. It also computes the interest to be paid at the end of the transaction expiration time, and only if the cover amount covers both interest and the balance to be covered it also adds a deposit operation for the collateral.

After the cover tx has been included in the blockchain, the cover operation is executed by cover_operation::evaluate. This computes the interest owed at the current block time, and if the cover amount covers both interest and principal the cover_order is deleted.

The problem here is the different times used for interest calculation. The default transaction expiration time is 1 hour, while the actual transaction evaluation takes place only a few seconds after the transaction was created. The transaction builder computed the interest owed one hour in the future and came to the conclusion that the cover amount was not sufficient, and therefore it did not create the deposit_operation for the collateral. The transaction evaluation calculated the interest owed now, saw the the cover amount was sufficient and closed the cover_operation.

[vikramrajkumar]

I have not confirmed the details of your theory yet, but it looks at least close to correct. And the collateral in that transaction indeed seems to have gone to fees.

Related: #1261

[pmconrad]

Interest owed after 170 seconds, calculated by cover_operation:

0.09825190 * .0012 * 170 / 365 / 24 / 60 / 60 = .0000000006355716 < 1 satoshi

Interest owed after 3600 seconds, calculated by tx builder:

0.09825190 * .0012 * 3600 / 365 / 24 / 60 / 60 = .0000000134591643 > 1 satoshi

[nmushegian]

This is the third time this has happened that I know of. This is a wallet fix that we could have had for 6.0.

Is dan going to keep covering these out of pocket?

[pmconrad]

I have scanned the blockchain for suspicious cover transactions (i. e. orders that fill the cover completely but don't have a deposit_operation included):

tx-id ... Collateral in .00001 BTS
ad834d88d0182437877b76cd9441fef8473282c0 9900327696
ae650e10d059294a8650d6dc9a416f9f221d652c 1519714081
25e0507e85f9327b3e278bbc74ecb0a895f3532b 842762272

I had a lot more hits, but the collateral was always below the default tx fee, so these are OK I suppose.

So you have a fix available? - Edit: I see, issue 1261

[vikramrajkumar]

@pmconrad I have made the simplest change which is to calculate interest at construction as it would be in 1 block interval from the current head block. This means that the full debt at confirmation should never be more than assumed at the time of construction. So when fully covering, either the full debt should be paid or the transaction should fail to confirm.

However, it occurs to me that it could be possible that the transaction ends up in an earlier block if there is some bad forking occurring, and so we are stuck with the same problem.

The intent of the new pay_fee op mentioned in #1216 is to explicitly specify the intended fees to be paid to the network, so that a transaction will fail to confirm if something else happens, such as in the above covers. I am open to better suggestions if you have any.

[vikramrajkumar]

Actually, it seems to me that we can just change the behaviour of the cover operation to directly return the collateral back to a balance record rather than expecting the transaction to collect it with a deposit.

[pmconrad]

Actually, it seems to me that we can just change the behaviour of the cover operation to directly return the collateral back to a balance record rather than expecting the transaction to collect it with a deposit.

IMO that would be the cleanest solution.

[vikramrajkumar]

I agree; it does complicate transaction scanning somewhat but still worth it.

[abitmore]

The 9900327696 one is this: https://bitsharestalk.org/index.php?topic=13075.0

[vikramrajkumar]

On second thought, for now I will hold off on changing the semantics of the operation in the interest of not adding another special case to transaction scanning. Instead I have added the limit_fee op when submitting a cover to make sure you don't pay any unintentional base fee. Eventually I intend to always include this op when constructing transactions.

[vikramrajkumar]

List of the bad covers:

[1465854] ad834d88d0182437877b76cd9441fef8473282c0 | 9900337696
[1784071] ae650e10d059294a8650d6dc9a416f9f221d652c | 1519724081
[1787759] 25e0507e85f9327b3e278bbc74ecb0a895f3532b | 842772272
[1997389] 84ea9a317e33bbf92857c2cf625790c3314385fc | 38224355405
[1997761] 7591cbcd1004d2ce004ab879d053758151858fc3 | 38223298975
[2033726] 1ab1948ec79dafbbf01f6c1a4433d46cdc00c683 | 5881752070
[2033744] e7c2b2dcfde27106b3fd09490b2dc7b05c7a7663 | 6533027197
[2035009] eca547196fe581fd5a8df7039c1798645c5dc4ee | 181666
[2035524] 283904505e023aec2a1476482f5c3fbced4690d2 | 1292717
[2035530] fcada16c256cfebcb84e0c5f35619ee6ee3d6797 | 1078931
[2035536] 8b8453e1832365218751883308f6e46ad3bfbcf0 | 531511
[2035542] 1302ab731c01421a030639566208254eb0d9ae04 | 792457
[2035544] 60f29e25df60a2d608d9b4bfed7fadefec746710 | 1267062
[2035551] 8ae29dfebbfc9c4f1034383d103e39260ee30fc5 | 1366489

For a total of 1,011,317.78529 BTS.

[abitmore]

For a total of 1,011,317.78529 BTS.

Refund planned?

[vikramrajkumar]

Yes, will be coordinated soon once we confirm the new release is okay.