[puLL-Merge] - brave/brave-core@32579

Description

This PR refactors the Cardano transaction fee and amount validation logic. The main changes include:

• Moves fee and output amount calculation from CardanoCreateTransactionTask to CardanoTransactionSerializer
• Replaces manual fee calculation loops with a centralized AdjustFeeAndOutputsForTx method
• Adds explicit fee_ field to CardanoTransaction instead of calculating it implicitly
• Introduces ValidateAmounts method to ensure transaction inputs equal outputs + fees
• Updates validation to occur in the solver layer rather than at task level
• Changes validation from runtime errors to CHECK assertions in critical paths

Possible Issues

1. Breaking Change in Validation: The PR changes when and how validation occurs. Previously, ValidateMinValue on target output happened in CardanoCreateTransactionTask, now it happens in the solver. This could silently break if solvers aren't updated correctly.

2. CHECK vs Error Handling: The code replaces error handling with CHECK assertions in CardanoCreateTransactionTask::RunSolverForTransaction(). If ValidateAmounts fails, the application will crash rather than returning an error to the user:

   CHECK(CardanoTransactionSerializer::ValidateAmounts(
       *solved_transaction, *latest_epoch_parameters_));

3. Overflow Handling: The PR changes from ClampSub to CheckedNumeric, which is safer, but the error handling paths may not be complete. Several places return std::nullopt on overflow but callers may not handle this gracefully.

4. Iteration Limit: The fee search loop has a fixed iteration limit (kFeeSearchMaxIterations = 10). If this is exceeded, the function returns nullopt without logging, making debugging difficult.

Security Hotspots

1. Integer Overflow in Fee Calculation (Medium Risk): The AdjustFeeAndOutputsForTx method uses CheckedNumeric but doesn't validate all paths. If an attacker can craft inputs that cause overflow in unguarded paths, they might be able to create transactions with incorrect fees:

   if (!base::CheckSub(total_inputs_amount, result.fee())
            .AssignIfValid(&result.TargetOutput()->amount)) {
     return std::nullopt;
   }

   The nullopt return could be silently ignored by callers.

2. CHECK-based Validation (Low-Medium Risk): Using CHECK for validation in production code means malformed transactions could crash the wallet rather than failing gracefully:

   CHECK(CardanoTransactionSerializer::ValidateAmounts(
       *solved_transaction, *latest_epoch_parameters_));

   This could be exploited for DoS if an attacker can trigger these conditions.

3. Fee Manipulation Risk (Low Risk): The iterative fee calculation approach could potentially be exploited if the iteration limit is reached. An attacker might craft inputs that require more than 10 iterations, causing transaction creation to fail.

Privacy Hotspots

1. Test Data Updates: The PR updates test UTXOs to use 969750 (minimum UTXO) and 2000000 instead of 54321 and 600000. While these are test values, they should not reflect real user patterns or be derived from production data.

2. Transaction Serialization Changes: The serialization format has changed to include explicit fee values. This could affect transaction privacy if the fee calculation method reveals information about the wallet implementation version.

sequenceDiagram
    participant Task as CardanoCreateTransactionTask
    participant Solver as CardanoKnapsackSolver/MaxSendSolver
    participant Serializer as CardanoTransactionSerializer
    participant Tx as CardanoTransaction
    
    Task->>Solver: Create with base tx + inputs
    Note over Solver: Validate target output min value
    Solver->>Solver: RunSolver iteration loop
    loop For each input combination
        Solver->>Serializer: AdjustFeeAndOutputsForTx(tx, params)
        Serializer->>Serializer: Calculate initial fee
        loop Max 10 iterations
            Serializer->>Tx: Adjust output amounts based on fee
            Serializer->>Serializer: CalcMinTransactionFee(tx)
            alt Fee converged
                Serializer->>Serializer: ValidateAmounts(tx)
                Serializer-->>Solver: Return valid tx
            else Need larger fee
                Note over Serializer: Increase fee, retry
            end
        end
    end
    Solver-->>Task: Return best solution
    Task->>Serializer: CHECK ValidateAmounts(tx)
    Task->>Task: Store transaction

📋 Code Owners Summary

17 file(s) changed

• 17 with assigned owners

1 team(s) affected: @brave/crypto-wallets-core

Owners and Their Files

@brave/crypto-wallets-core — 17 file(s)

• components/brave_wallet/browser/cardano/cardano_create_transaction_task.cc
• components/brave_wallet/browser/cardano/cardano_create_transaction_task_unittest.cc
• components/brave_wallet/browser/cardano/cardano_get_utxos_task_unittest.cc
• components/brave_wallet/browser/cardano/cardano_knapsack_solver.cc
• components/brave_wallet/browser/cardano/cardano_knapsack_solver_unittest.cc

Are we changing how the fee structure works for smart contract execution transactions?

From what I can see, the calculation here operates a bit differently and I don't see anything about the way that Collateral requirements work or execution budget aspects.

The concern here would be that the fee structure isn't correct and lead to a phase 2 failure where a small fee (the collateral) is still collected even though the transaction didn't execute properly. Here's some docs I found that highlight how this works: https://www.essentialcardano.io/article/no-surprises-transaction-validation-part-2

Does that have an impact on this PR at all?

Phase 2 failure fee is burned from collateral inputs which we don't(and didn't) add to transactions.

Released in v1.87.47