feaT:refactor event + add more info in parse pm data #30
Conversation
| } else { | ||
| // here chargedAmount accounts for penalty with maxGasPenalty | ||
| emit GasBalanceDeducted(paymasterId, prechargedAmount, premium); |
There was a problem hiding this comment.
can it really happen?
if prechargedAmount < adjustedGasCost doesn't paymaster lose money?
if we only expect else to mean prechargedAmount == adjustedGasCost, we do need else at all
There was a problem hiding this comment.
I feel it would never go into the else case, but if it does then want to emit different value in the event.
There was a problem hiding this comment.
if we pre charged less than we want to charge more yeah. in that case otherwise paymaster loses money. so diff of that adjusted - pre charged should be deduced from paymasterId balance.
There was a problem hiding this comment.
Makes sense. Does it make sense to emit different event to easily recognize we charged less
There was a problem hiding this comment.
Can do it but it will be overhead on the paymaster service.
so, should I leave this
- as it is
- as it is but different event name
- same/different event name but in the else case doing
paymasterIdBalances[paymasterId] += (adjustedGasCost - prechargedAmount);
There was a problem hiding this comment.
There was a problem hiding this comment.
going with 3 with same event name.
| // deduct what needs to be deducted from paymasterId | ||
| paymasterIdBalances[paymasterId] -= (adjustedGasCost - prechargedAmount); | ||
| // here chargedAmount accounts for penalty with maxGasPenalty | ||
| emit GasBalanceDeducted(paymasterId, prechargedAmount, premium); |
There was a problem hiding this comment.
I believe you should emit adjustedGasCost in this case as it is now what in fact was charged after L289 right?
so emit can be moved out of if/else
There was a problem hiding this comment.
right. with this addition it makes sense
I wanted to emit different things that's the whole reason I added if else.
but it's ok.
knowing that charge can never be accurately emitted here and will need some off-chain component for penalties.
No description provided.