Skip to content

feat: adds better tracking of funds going to treasury #576

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ckartik
Copy link
Contributor

@ckartik ckartik commented Jan 22, 2025

  • Improves traceability of funds going to treasury
  • Improves slashing traceability based on commitment digest.

@ckartik ckartik requested review from shaspitz and Mikelle January 22, 2025 18:32
@ckartik ckartik requested a review from shaspitz January 23, 2025 00:08
Copy link
Contributor

@shaspitz shaspitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved given we add a todo, or track the task of removing the old events once everyone's migrated to a new version

@@ -126,7 +128,7 @@ contract ProviderRegistry is
bidderSlashedAmount[bidder] += residualAmt;
}

emit FundsSlashed(provider, residualAmt + penaltyFee);
emit FundsSlashed(provider, residualAmt + penaltyFee, commitmentDigest);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I thought what we discussed is emitting the new event and not changing the old one. If we change the old one I am not sure how backward compatibility works. For eg, if they listen to old contract events with the new ABI what would happen? Would it be able to fill only the relevant items?

@@ -10,7 +10,7 @@ interface IProviderRegistry {
event FundsDeposited(address indexed provider, uint256 amount);

/// @dev Event emitted when funds are slashed
event FundsSlashed(address indexed provider, uint256 amount);
event FundsSlashed(address indexed provider, uint256 amount, bytes32 indexed commitmentDigest);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add a new field which is indexed I am not sure if it will be able to Watch/Filter this event if we listen for it for block numbers before the contract update was made. This might be a problem for indexing the chain data using other tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants