Feature/Fix: Add simulated transaction fee to calculations.csv and fix compute logic

[vkresch]

Partial fix for #455
Resolves #567
Resolves #565
Resolves #441
Resolves #512
fixes #430
Proper error message in description fixes #270

The payments have been correct for all tz accounts. kt accounts with burn fees have been incorrect. ONLY the reporting had major bugs in the .csv report. The overall terminal logs have been correct. Reported overall payout amount has been incorrect.

General:

• add simulated transaction fees into the calculations report csv: delegate_transaction_fee; delegator_transaction_fee
• introduced function to modify the calculations csv
• added description field to the payment .csv report
• using user friendly enum names for the payment status now
• fixed a LOT of bugs in the payments report
• changed all occurrences of XTZ and tez to mutez properly as formatted int( ) for consistency
• added explicit type casts all over the code
• removed dead code that allowed for kt accounts to be used as payout address
• added error messages in payment csv for unsuccessful transactions
• added documentation on how to test with vs code
• added requirements for development
• removed magic numbers from the code and added them to Constants.py
• removed strings and replaced them with existing constants
• harmonized function names

TODO:

• add both columns to the calculations.csv and not the payments.csv
• fix tests (disabled it)

Fix:

• burn fee of kt accounts was always substracted from the delegator payment amount. It is now also depended on the configuration delegator_pays_ra_fee (Reactivation fee), fixed documentation
• Reported amount to pay was an estimate and not calculated properly with regard on what has been paid or skipped and the actual fees
• the payment_logs had missing items regarding items with the status AVOIDED
• tayments that are below zero_threshold are now set to status DONE according to the definition
• payment_items had been modified but never been added properly to payment_logs and therefore been discarded
• added issue for reporting error regarding rules_map
• fixed args_parser_validation function
• adopted webhook plugin for adjusted_amounts replacing amounts in reports
• fixed bug that lead to exclusion of 0 amount payments in payments csv
• fixed conversion bugs in calculation files parser functions
• added function to determine early_payout mode in calculations file

Tests:

• test_attempt_single_batch_tz which test if the transaction_fee gets calculated correctly
• test_gas_estimation_oven_kt1.pyis now failing as the RPC call never worked
• fixed webhook test
• fixed file parser test

Note:

• Refactoring regarding the naming convention of variables. I think we should try naming variables with its full names to be explicit.
• Removed comments which do not provide any new information.
• Added comments to help understand fee simulation

vkresch, Effort=6h
amzid, Effort=2h
jdsika, Effort=32h (from other branch as well)

Remark: jdsika spent hours on phone calls and messages to figure out all the broken parts here and fix them in the PR as well as hours debugging reporting errors.

[nicolasochem]

this conflicts with adjusted early payouts, but not in a big way for now.

[jdsika]

I agree on the renaming of the variables. They need to be expressive. Did you test it with example cycles of certain bakers? A normal transaction is not calculated, right (tz to tz address). Only tz to kt? Is the sum of fees the overall fee of a batch and do you just divide the sum through the items in the batch?

[jdsika]

@nicolasochem should we merge this PR before? The impact on the code is rather minor compared to yours? :)

[jdsika]

@vkresch batch_payer.py has the constants for fee limits. As I read it they are taken as "lowest value" to start every transaction with and adjust the fee in the simulation process. It will be interesting to see what the descrepancy between the set fee and the actual consumed fee will be.

[nicolasochem]

This PR fills the done payment reports with a value taken from batch_payout.py. For the payout to tz addresses, this value is TZTX_FEE of 395 mutez, for a payout to a kt1 address it is TZTX_FEE + COST_PER_BYTE * storage limit if I read the code correctly.

I am not very familiar with tezos fee mechanisms and it is the first time I look at this logic, so beware.

I think I need a better user story than what's currently on #455 to review this. It says "for accounting purposes".

You can configure TRD to pay the fee or take it away from the delegator. Does #455 assume that the bakery pays the fee, so this value is needed for bakery accounting?

If this is the case, should this new column not be filled with zeroes when the baker takes the fee away from the delegator? Or should we have 2 columns "fees_paid_by_delegate" "fees_paid_by_delegator"

Second, this PR populates the column with fee estimates. But the ticket says "actual fees". Is this PR the prelude to another one?

I think it is not possible to accurately estimate the fee for a KT1 payout because the actual fee depends on all transactions in the bloc, not just the one you are signing. But TRD estimates the fee for a KT1 payout and substracts it if configured to do so. Do we want to use this estimate in the payout done column? Is it not better to look at the inclusion block and put the actual fee paid in there? (if at all possible)

@jdsika please clarify

[jdsika]

I apologize for the bad user story. You are absolutely right that it is not sufficient!

If this is the case, should this new column not be filled with zeroes when the baker takes the fee away from the delegator? Or should we have 2 columns "fees_paid_by_delegate" "fees_paid_by_delegator"

Yes, I absolutely agree with this. I had the same conversation with @dansan566

Second, this PR populates the column with fee estimates. But the ticket says "actual fees". Is this PR the prelude to another one?

Yes, this should be part of the calculations and we need to introduce a post processing step if needed in the payments report later.

Steps:

1. Check if the method of fee calculation in general makes sense and if the options in the configuration are correctly applied (as I read over the code I suspected an error regarding the fee substraction for kt accounts)
2. Put the estimated fees for delegate and delegator in separate columns in the calculations report (I will adapt the user story in Request: Transaction fee necessary in report #455)
3. Create a new ticket for fetching actual fees in a post processing step and add them to the payments report

[nicolasochem]

ok, so in the absence of bugs, do we want this merged as is?

[jdsika]

ok, so in the absence of bugs, do we want this merged as is?

Look at my comment. I think there ARE bugs or at least inconsistencies...

1. @vkresch The simulated fees should be in the reports_dir/calculations/ ....csv file. It needs two columns. The fees paid by the baker and the fees payed by the client.
2. we need to figure out if the whole thing makes sense at all...

What did I find on transaction costs:
Knowledge base
StackExchange with link to Tezos Docs as well

[nicolasochem]

Yes, simulated fee belongs to the calculations csv. Note that instead of 2 columns (paid by delegate/paid by delegator) we could have a column with uppercase letter B or D where B indicates "paid by baker" and D indicates "paid by delegator). This is similar to the convention used in payment_type.

Some months ago I raised a bug about reactivate_zeroed generating invalid transactions (#508) so I think as well that there is something wrong with the way we calculate fees.

I am not quite sure what to do with this PR. Perhaps involve the original author of the fee logic? Maybe let's not touch this until tenderbake is done?

[jdsika]

@nicolasochem I just talked to @amzid and he will fix the errors at the weekend!

[jdsika]

@utdrmac I disabled the test now. Otherwise we will never get things running for Tenderbake

[nicolasochem]

I tried this code out by merging it in my ithaca branch.

I saw in the calculation csv that there are 2 new columns delegate_transaction_fee and delegator_transaction_fee:

ee_rate | overestimate | adjustment | adjusted_amount | delegate_transaction_fee | delegator_transaction_fee | payable | skipped | atphase | desc   >
------- | ------------ | ---------- | --------------- | ------------------------ | ------------------------- | ------- | ------- | ------- | ------->
   0.00 |        False |      False |   2,755,193,368 |                    5,606 |                     1,975 |   False |   False |      -1 | Baker  >
   0.05 |        False |      False |     204,035,376 |                    5,606 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.00 |        False |      False |   1,967,688,412 |                        0 |                         0 |   False |   False |      -1 |  Not in>
   0.00 |        False |      False |      39,375,248 |                        0 |                         0 |   False |   False |      -1 |  Not i

Normally either the delegate or the delegator pay the fee, but here I can see that for my first payout, both delegator and delegate fee are positive. How come?

[jdsika]

I tried this code out by merging it in my ithaca branch.

I saw in the calculation csv that there are 2 new columns delegate_transaction_fee and delegator_transaction_fee:

ee_rate | overestimate | adjustment | adjusted_amount | delegate_transaction_fee | delegator_transaction_fee | payable | skipped | atphase | desc   >
------- | ------------ | ---------- | --------------- | ------------------------ | ------------------------- | ------- | ------- | ------- | ------->
   0.00 |        False |      False |   2,755,193,368 |                    5,606 |                     1,975 |   False |   False |      -1 | Baker  >
   0.05 |        False |      False |     204,035,376 |                    5,606 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.05 |        False |      False |     136,023,583 |                        0 |                       395 |    True |   False |      -1 |        >
   0.00 |        False |      False |   1,967,688,412 |                        0 |                         0 |   False |   False |      -1 |  Not in>
   0.00 |        False |      False |      39,375,248 |                        0 |                         0 |   False |   False |      -1 |  Not i

Normally either the delegate or the delegator pay the fee, but here I can see that for my first payout, both delegator and delegate fee are positive. How come?

The first entry is your baker which displays the sum of all values below. The same way as it sums the overestimate e.g.

BTW there are two options in the config:

1. delegator_pays_xfer_fee
2. delegator_pays_ra_fee

You can set one to True and the other to False and if there are burn fees then both columns in a row are populated.

[jdsika]

@nicolasochem maybe we have to check for the standard fees here

[Pea-skillz]

Calculations look good between the TRD used in production and this branch. I will run the next payment on this branch (i just need to adjust the changes related to the config reorganisation).