improve: Destructure RelayData params in FilledRelay event #18
Conversation
|
|
||
| struct RelayData { | ||
| address sender; | ||
| address depositor; |
There was a problem hiding this comment.
Unrelated: naming this to depositor is more clear and better delineates between "sender" and "relayer"
chrismaree
left a comment
chrismaree
left a comment
There was a problem hiding this comment.
does this change the gas cost of the emitting?
contracts/SpokePool.sol
Outdated
| uint256 totalRelayAmount, | ||
| uint256 newFilledAmount, | ||
| uint256 indexed repaymentChain, | ||
| uint256 amountSentToRecipient, |
There was a problem hiding this comment.
Instead of reporting how much they sent to the recipient, do you think we should report how much was added to the filled amount? We have the newFilledAmount, but not the diff. Seems like it might be easier (from a bot perspective) to compute how much the relayer is owed from that amount rather than the amountSent. Thoughts?
There was a problem hiding this comment.
Hmm I think you might be right. @chrismaree do you have instincts on this? The relayer would be able to compute (1 + relayerFeePct) * fillIncreaseAmount. Question though is whether they even would care about this new fill amount. Also, should we simply append the fill delta or replace the amountSentToRecipient?
There was a problem hiding this comment.
The relayer would be able to compute
(1 + relayerFeePct) * fillIncreaseAmount.
I may be wrong, but I think the formula for the repayment amount would be (1 - lpFeePct) * fillIncreaseAmount. Does that seem right to you?
There was a problem hiding this comment.
I guess you could compute amountSentToRecipient by calculating (1 - relayerFeePct - lpFeePct) * fillIncreaseAmount
There was a problem hiding this comment.
I replaced amountToSend with filllAmountPreFees and renamed the param to fillAmount
Empirically it doesn't look like it changes anything @chrismaree |
mrice32
left a comment
mrice32
left a comment
There was a problem hiding this comment.
LGTM once we resolve this thread: #18 (comment).
In the past, emitting a complex object in an event causes Etherscan difficulty in decoding the params. We should emit all neccessary params individually.