WithdrawReason unused.

So I think I am missing something about the nature of the lock here that this strikes me as counter-intuitive. To me, this account should now have all of its 2*1280 units of fund locked and the transfer should fail. The implementation of ensure_account_can_withdraw seems also close to what I assumed but something is missing in my mind here.

following up on the above: shouldn't a lock affect free-vesting balance?
In other words what would have made this test (of course for me) more understandable was to see the operation and then a query afterwards on how it affected the different balance storage items corresponding to 1:

Balances::set_lock(ID_1, &1, 1280, u64::max_value(), WithdrawReasons::all());
Balances::set_lock(ID_2, &1, 1280, u64::max_value(), WithdrawReasons::all());

// knowing that initially free = 2580 and vesting = 0
assert_eq!(Balances::free_balance(&1), XXX); 
assert_eq!(Balances::vesting_balance(&1), YYY); 

@joepetrowski since I see that you are assigned to the documentation of this module this might also be related to you.

Given that we decided to put as much doc as possible into the code files, documenting these might also be helpful.

Should this be moved to Currency trait or LockableCurrency trait?

No, because it is used within the module (pub fn make_transfer) to ensure that there is enough unvested balance. All chains using the balances module allow vesting, so this check makes sense.

Regarding the use of locks, Locks is a storage item of the entire module. If the logic makes it past the vesting check and the chain does not implement Currency or LockableCurrency then the locks getter will return empty and the function will return Ok(()).

let locks = Self::locks(who);
if locks.is_empty() {
	return Ok(())    // <--- Will return here
}

I mean it is better to also expose ensure_account_can_withdraw in the Currency or LockableCurrency trait so other modules are able to check this liquidity before calling mutate method offered by Currency trait.

what is the reason to include this unused amount as well as new_balance?
it should be able to calculate one from other so why passing it twice?
also it doesn't query user free balance and check amount is greater than that.
I found it is awkward to use this call. I need to check free balance is more than amount, and calculate the new amount, and call this method check to check if a transfer is allowed.
Shouldn't this does all the check and query for me?

Missing check?
Self::ensure_account_can_withdraw(transactor, fee, WithdrawReason::TransactionPayment, new_from_balance)?
fee is used for transaction payment and value is used for transfer.
This means ensure_account_can_withdraw needs to be able to designed to be able to query multiple reasons at once?
Something like
Self::ensure_account_can_withdraw(transactor, vec![(fee, WithdrawReason::TransactionPayment), (value, WithdrawReason::Transfer)])?

or Self::ensure_account_can_withdraw(transactor, WithdrawReason::TransactionPayment | WithdrawReason::Transfer, new_from_balance)?