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

Fix reward payment tx fee calculation #427

Merged
merged 6 commits into from
Nov 20, 2023

Commits on Nov 15, 2023

  1. pool: Split applyTxFees tests from PaymentMgr test

    This breaks up the monolithic testPaymentMgrPayment, extracting the bits
    specific to testing the applyTxFees func. The goal is to make updating
    the tests less cumbersome.
    
    Two new tests are introduced, one for the core applyTxFees
    functionality, and one for its error conditions.
    jholdstock committed Nov 15, 2023
    Configuration menu
    Copy the full SHA
    be98a5f View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    2942837 View commit details
    Browse the repository at this point in the history
  3. pool: Remove output total param from applyTxFees.

    The applyTxFees func previously took in a set of outputs as one param,
    and the total value of those outputs as a separate param. This
    introduced an opportunity for the function to be misued in that it could
    be passed a set of outputs and a total which do not match.
    
    An instance of this mistake actually existed in the tests for
    applyTxFees. The value of the inputs and outputs was 100 DCR, but the
    provided total was 5 DCR.
    jholdstock committed Nov 15, 2023
    Configuration menu
    Copy the full SHA
    5127213 View commit details
    Browse the repository at this point in the history

Commits on Nov 16, 2023

  1. pool: Simplify and parameterize applyTxFees test.

    The test can now be run multiple times with different values so that
    applyFee can be tested more thoroughly. This also enables characterizing
    the existing behaviour of applyTxFees. Using absolute values in the
    tests means that assertions are more precise, and any issues are more
    clearly visible.
    jholdstock committed Nov 16, 2023
    Configuration menu
    Copy the full SHA
    5072c6c View commit details
    Browse the repository at this point in the history
  2. pool: Extract fee estimation func.

    This extracts fee estimation into a func which can be reused later, as
    well as fixing an issue which caused inSizes and outSizes to be twice as
    long as intended, leading to fees being overestimated.
    jholdstock committed Nov 16, 2023
    Configuration menu
    Copy the full SHA
    ffad0c9 View commit details
    Browse the repository at this point in the history

Commits on Nov 17, 2023

  1. pool: Fix logic in applyTxFees.

    There was a couple of problems in applyTxFees which lead to fees being
    greatly overestimated.
    
    - The division to find the ratio of a single output to the total output
      amount was inverted.
    - The pool fee should not be subject to paying fees, however the txfee
      was being subtracted from the total output amount rather than the
      pool fee.
    
    A new assertion in the applyTxFees test ensures the deductions from
    output amounts matches matches the expected fee to be paid.
    jholdstock committed Nov 17, 2023
    Configuration menu
    Copy the full SHA
    9b692c8 View commit details
    Browse the repository at this point in the history