internal/ethapi: return detail insufficient fund error in EstimateGas #28161
Conversation
Signed-off-by: jsvisa <delweng@gmail.com>
| value := args.Value.ToInt() | ||
| if value.Cmp(available) >= 0 { |
There was a problem hiding this comment.
Hm, isn't it odd that the comparison (now and before) uses >=, as opposed to just > ? Why error on equality?
There was a problem hiding this comment.
IMHO, as feeCap is not zero, we need to consider the gasFee, so = is not enough, maybe I need to adjust the error message.
There was a problem hiding this comment.
BTW, I think we should always check value with balance, no matter feeCap is zero or not, in that case = should be omited.
There was a problem hiding this comment.
Changed the error from core.ErrInsufficientFundsForTransfer to core.ErrInsufficient(the later contains the gas*price part)
There was a problem hiding this comment.
Why not ErrInsufficientFundsForTransfer? This clause is specifically comparing the value against the balance.
There was a problem hiding this comment.
As ErrInsufficientFundsForTransfer only considered the transferred value's part, not including the gasFee, and here as the feeCap is not zero, we need to add the feeCap. So ErrInsufficientFunds probably more accurate, because his error log is insufficient funds for gas * price + value
Signed-off-by: jsvisa <delweng@gmail.com>
Signed-off-by: jsvisa <delweng@gmail.com>
Signed-off-by: jsvisa <delweng@gmail.com>
| return 0, core.ErrInsufficientFundsForTransfer | ||
| value := args.Value.ToInt() | ||
| if value.Cmp(available) >= 0 { | ||
| return 0, fmt.Errorf("%w: address: %s, have: %v, want: %v+(gas)", core.ErrInsufficientFunds, *args.From, available, value) |
There was a problem hiding this comment.
I don't know why it says (gas) ? The unit is wei
| return 0, fmt.Errorf("%w: address: %s, have: %v, want: %v+(gas)", core.ErrInsufficientFunds, *args.From, available, value) | |
| return 0, fmt.Errorf("%w: address: %v, have: %v, want: >%v", core.ErrInsufficientFundsForTransfer, *args.From, available, value)) |
There was a problem hiding this comment.
Sorry, I intended to say that there was an extra gasFee here. maybe changed into something like below:
- return 0, fmt.Errorf("%w: address: %s, have: %v, want: %v+(gas)", core.ErrInsufficientFunds, *args.From, available, value)
+ return 0, fmt.Errorf("%w: address: %s, have: %v, want: %v+(gasFee)", core.ErrInsufficientFunds, *args.From, available, value)
No description provided.