-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(protocol): fix issues in AssignmentHook #15486
Conversation
- add chainId to hashed assignment - prevent replay and then repay of bind in the same assigmenthook array
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
It is not used in this hook, but maybe used in another. |
// have increased by the same amount as config.livenessBond (to prevent) | ||
// multiple draining payments by a malicious proposer nesting the same | ||
// hook. | ||
if (tko.balanceOf(address(this)) != tkoBalance + config.livenessBond) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check TKO balance change strict using == rather than >=.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adaki2004 with this change, maybe we don't have to check "==" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still recommend having a strict equality, for two reasons
a) I don't see a benefit for transferring more than the liveness bond, it would increase the cost to the prover and the extra funds would be stuck in the TaikoL1 contract.
b) If multiple hooks are available which can transfer TKO to the TaikoL1 contract the proposer could attempt to use both e.g. If there is a third party hook, a proposer could attempt to use both hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still recommend having a strict equality, for two reasons
I think this is a strict equalitsy, no ? (if NOT -> revert)
if (tko.balanceOf(address(this)) != tkoBalance + config.livenessBond) revert L1_LIVENESS_BOND_NOT_RECEIVED
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep this PR currently has a strict equality, which I currently see as a nicer solution. I was responding to @dantaik comment about potentially moving back to inequality.
@adaki2004 with this change, maybe we don't have to check "==" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And client updates are ready too: taikoxyz/taiko-client#502
From team SigP.:
==
rather than>=
.