feat(contracts): unbond validator KRO #351

seolaoh · 2024-07-19T08:06:33Z

Description

Unbond validator KRO during output finalization and slashing.

Additionally, there is a point to consider related to challenge process. I wanna discuss about the way to handle it, which I implemented in 83f1908.

When a malicious challenger successfully proved fault due to a ZK bug, the asserter will be slashed and not have bond anymore. But after challenge dismissed by security council, the output submitter is reverted to the asserter and the asserter's asset will be tried to be unbonded when output being finalized. In that case, the asserter does not have bond since it has been slashed, so output finalization will be reverted.

I made a change to replace the output submitter to security council when dismissing challenge, and block proving fault when submitter is security council because the output has already been validated by security council.

Please share your thoughts about the solution is appropriate or not.

…slashing

When a malicious challenge successfully proved fault due to a ZK bug, the asserter will be slashed and not have bond anymore. But after challenge dismissed by security council, the output submitter is reverted to the asserter and the asserter's asset will be tried to be unbonded when output being finalized. In that case, the asserter does not have bond since it has been slashed, so output finalization will be reverted. I made a change to replace the output submitter to security council when dismissing challenge, and block proving fault when submitter is security council because security council has already validated the output.

coderabbitai · 2024-07-19T08:06:39Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Base branches to auto review (2)

dev
main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

packages/contracts/contracts/L1/ValidatorManager.sol

sm-stack · 2024-07-22T04:32:03Z

packages/contracts/contracts/L1/Colosseum.sol


 // Rollback output root.
- L2_ORACLE.replaceL2Output(_outputIndex, _outputRoot, _asserter);
+ L2_ORACLE.replaceL2Output(_outputIndex, _outputRoot, SECURITY_COUNCIL);


As I understand, this solution to replace the submitter to security council in the case of ZK bug gives

challenge reward to asserter

output reward to security council

no bond back to the asserter

Is this right? Then I think it's somewhat unfair to give the output reward to security council, since it was actually the fund that should go to the asserter if there was no ZK related bug. How about making an extra function like recoverAsserterBond() that recovers asserter's bond back inside the below if statement?

Yes, what you listed is almost correct, but challenge reward will also be transferred to security council.

In this way, we need to refund the asserter's slashed bond + challenge reward + output reward to the asserter manually. In V1, we also need to refund the tax to the asserter manually in dismissing challenge case.

I also considered adding recoverAsserterBond, but the tax amount is already tranferred to security council during slashing, so we cannot recover the asserter's bond fully. But in output finalization, the full bond amount should be unbonded, so there are also some difficulties in that way.

Yes, what you listed is almost correct, but challenge reward will also be transferred to security council.

Oh yeah that's right.

How about transferring tax back to the AssetManager from security council in the case of dismissChallenge? Is there any context at the time we decide not to return the tax back to ValidatorPool at V1?

There was no context at that time, since we did not actually undergo such cases.

Hmm then how about transferring the tax at the time of output finalization? Just add BOND_AMOUNT to _pendingChallengeReward and handle it after all the challenges are concluded.

@0xHansLee wdyt about this issue?

That seems better imo

I applied it, please take a look.

It's reasonable to me.
BTW, when the challenge is dismissed, should the asserter be refunded? Once the challenge is proven, the asserter get slashed. I think asserter should be refunded the bond that already slashed.

Oh I missed the part. I'll add recoverAsserterBond function.

packages/contracts/contracts/L1/AssetManager.sol

sm-stack

LGTM

0xHansLee · 2024-07-22T09:00:05Z

packages/contracts/contracts/L1/interfaces/IValidatorManager.sol

Please add SlashReverted event

seolaoh added 2 commits July 19, 2024 17:01

feat(contracts): unbond validator KRO during output finalization and …

18282e6

…slashing

seolaoh self-assigned this Jul 19, 2024

seolaoh requested a review from a team as a code owner July 19, 2024 08:06

This was referenced Jul 21, 2024

feat(contracts): update validator reward calculation #355

Merged

feat(contracts): add transferFrom when increasing reward of vault #354

Merged

sm-stack reviewed Jul 22, 2024

View reviewed changes

feat(contracts): apply PR reviews

f2e1a11

seolaoh requested a review from sm-stack July 22, 2024 05:06

0xHansLee reviewed Jul 22, 2024

View reviewed changes

packages/contracts/contracts/L1/AssetManager.sol Outdated Show resolved Hide resolved

seolaoh added 2 commits July 22, 2024 15:11

feat(contracts): handle tax when finalizing output

03fa2b2

feat(contracts): apply PR reviews

61c6d9a

sm-stack approved these changes Jul 22, 2024

View reviewed changes

feat(contracts): revert slash

6dfa1c7

seolaoh requested a review from 0xHansLee July 22, 2024 08:19

0xHansLee reviewed Jul 22, 2024

View reviewed changes

feat(contracts): add SlashReverted event to interface

25fd3b3

seolaoh requested a review from 0xHansLee July 22, 2024 09:02

0xHansLee approved these changes Jul 22, 2024

View reviewed changes

seolaoh merged commit b5b2ec9 into feat/implement-validator-system-v2 Jul 22, 2024
1 of 2 checks passed

seolaoh deleted the feat/unbond-validator-kro branch July 22, 2024 09:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(contracts): unbond validator KRO #351

feat(contracts): unbond validator KRO #351

seolaoh commented Jul 19, 2024

coderabbitai bot commented Jul 19, 2024

Review skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

sm-stack Jul 22, 2024 •

edited

Loading

seolaoh Jul 22, 2024

sm-stack Jul 22, 2024

seolaoh Jul 22, 2024

seolaoh Jul 22, 2024

sm-stack Jul 22, 2024 •

edited

Loading

seolaoh Jul 22, 2024

0xHansLee Jul 22, 2024

seolaoh Jul 22, 2024

sm-stack left a comment

0xHansLee Jul 22, 2024

feat(contracts): unbond validator KRO #351

feat(contracts): unbond validator KRO #351

Conversation

seolaoh commented Jul 19, 2024

Description

coderabbitai bot commented Jul 19, 2024

Review skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

sm-stack Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm-stack Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm-stack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodeRabbit Configuration File (`.coderabbit.yaml`)

sm-stack Jul 22, 2024 •

edited

Loading

sm-stack Jul 22, 2024 •

edited

Loading