Skip auto-funding for accounts that were explicitly dealt

[filip-parity]

Fix fund_pranked_accounts overwriting dealt account balances.
Track accounts explicitly funded via vm.deal() and skip auto-funding them in fund_pranked_accounts. Previously, pranking an account that had spent its balance to zero would incorrectly set it to u128::MAX, breaking balance assertions in tests.

[pkhry]

Can you add a repro test case?

[filip-parity]

Can you add a repro test case?

Added testcase for this

[pkhry]

what is going to happen when vm.deal was executed exclusively within foundry side?

[smiasojed]

how does it work in REVM if prank addresses do not have any balance?

[filip-parity]

It works, in REVM they don't need balance to execute

[filip-parity]

what is going to happen when vm.deal was executed exclusively within foundry side?

Balance changes don't sync back when executed in pallet-revive, it's a different issue though, we can look separately into this

[pkhry]

what is going to happen when vm.deal was executed exclusively within foundry side?

Balance changes don't sync back when executed in pallet-revive, it's a different issue though, we can look separately into this

they do.
the question was: what's going to happen if we call the vm.deal inside a callback?

ie

proxy.call() // calls the test contract, original scope
// inside test contract
vm.deal(addr1, 2 ether); // executed inside callback to original scope.
//
vm.prank(addr1)
addr1.balance // if the addr1 didn't exist on pallet-revive side it's balance would be 0. instead of the balance that was set inside the vm.deal() call.

but i digress, it can be changed in other PR.

[smiasojed]

hint from Claude AI, I did not check it in details but is worth to consider, I think:

pub(crate) fn fund_pranked_accounts(&self, account: Address) {
        // In pallet-revive, contracts need funds to cover storage deposits for their code.
        // If a pranked account is a contract with insufficient balance, it becomes non-functional
        // and any storage writes will fail with StorageDepositNotEnoughFunds.
        //
        // Unlike EVM where balance is just tracked, pallet-revive requires storage deposits
        // to be properly held (reserved) on the contract account. Just setting the free balance
        // is not enough - we must create the hold as would happen in a real transaction.
        //
        // Only fund if the pranked account is a contract.
        // EOAs don't need storage deposit funds since they don't execute code.
        let account_h160 = H160::from_slice(account.as_slice());

        // Check if account is a contract and get its info
        let Some(contract_info) = AccountInfo::<Runtime>::load_contract(&account_h160) else {
            return; // EOA or no contract - no funding needed
        };

        // Get the required deposit from contract info (includes code + storage deposits)
        let required_deposit: u128 = contract_info.total_deposit();
        if required_deposit == 0 {
            return;
        }

        // Convert to AccountId32 for balance operations
        let account_id = AccountId32Mapper::<Runtime>::to_fallback_account_id(&account_h160);

        // Convert HoldReason to RuntimeHoldReason
        let hold_reason: RuntimeHoldReason = HoldReason::StorageDepositReserve.into();

        // Fund the account with enough balance to cover the deposit + some buffer for ED
        // We need free balance available to then hold it
        let current_balance = Pallet::<Runtime>::evm_balance(&account_h160);
        let required_total = SpU256::from(required_deposit);
        if current_balance < required_total {
            Pallet::<Runtime>::set_evm_balance(&account_h160, required_total)
                .expect("Could not fund contract with required deposit");
        }

        // Create the hold on the contract account, as would happen in a real transaction.
        // This properly reserves the storage deposit funds.
        if let Err(e) = Balances::hold(&hold_reason, &account_id, required_deposit) {
            tracing::warn!(
                target: "cheatcodes",
                "Failed to hold storage deposit for contract {:?}: {:?}",
                account_h160,
                e
            );
        }
    }

[pkhry]

why do we need this field if we can just pass it to fund_pranked_accounts?

[filip-parity]

Addressed

[pkhry]

maybe we should check if the balance aligns between foundry's REVM and pallet-revive and then align them if they are divergent in case of e.g vm.deal execution within a callback as it will not set the balance within pallet-revive but will be present in eth_deals.

[filip-parity]

Also addressed, and we default to max if there is no deal

[filip-parity]

hint from Claude AI, I did not check it in details but is worth to consider, I think:

pub(crate) fn fund_pranked_accounts(&self, account: Address) {
        // In pallet-revive, contracts need funds to cover storage deposits for their code.
        // If a pranked account is a contract with insufficient balance, it becomes non-functional
        // and any storage writes will fail with StorageDepositNotEnoughFunds.
        //
        // Unlike EVM where balance is just tracked, pallet-revive requires storage deposits
        // to be properly held (reserved) on the contract account. Just setting the free balance
        // is not enough - we must create the hold as would happen in a real transaction.
        //
        // Only fund if the pranked account is a contract.
        // EOAs don't need storage deposit funds since they don't execute code.
        let account_h160 = H160::from_slice(account.as_slice());

        // Check if account is a contract and get its info
        let Some(contract_info) = AccountInfo::<Runtime>::load_contract(&account_h160) else {
            return; // EOA or no contract - no funding needed
        };

        // Get the required deposit from contract info (includes code + storage deposits)
        let required_deposit: u128 = contract_info.total_deposit();
        if required_deposit == 0 {
            return;
        }

        // Convert to AccountId32 for balance operations
        let account_id = AccountId32Mapper::<Runtime>::to_fallback_account_id(&account_h160);

        // Convert HoldReason to RuntimeHoldReason
        let hold_reason: RuntimeHoldReason = HoldReason::StorageDepositReserve.into();

        // Fund the account with enough balance to cover the deposit + some buffer for ED
        // We need free balance available to then hold it
        let current_balance = Pallet::<Runtime>::evm_balance(&account_h160);
        let required_total = SpU256::from(required_deposit);
        if current_balance < required_total {
            Pallet::<Runtime>::set_evm_balance(&account_h160, required_total)
                .expect("Could not fund contract with required deposit");
        }

        // Create the hold on the contract account, as would happen in a real transaction.
        // This properly reserves the storage deposit funds.
        if let Err(e) = Balances::hold(&hold_reason, &account_id, required_deposit) {
            tracing::warn!(
                target: "cheatcodes",
                "Failed to hold storage deposit for contract {:?}: {:?}",
                account_h160,
                e
            );
        }
    }

Should this be addressed in the scope of this PR?