Skip to content
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

Revisiting errors emitted by StakeMath #130

Open
0x-r4bbit opened this issue Feb 19, 2025 · 0 comments
Open

Revisiting errors emitted by StakeMath #130

0x-r4bbit opened this issue Feb 19, 2025 · 0 comments

Comments

@0x-r4bbit
Copy link
Collaborator

Basically opening this discussion again: https://github.com/vacp2p/staking-reward-streamer/pull/109/files#r1914773210

StakeMath currently throws errors.
This is generally not a bad thing but somewhat unexpected because it's just a library.

One case that strengthens this argument is that the code looks like there's missing checks in the staking manager.
Best example is here: #126

I've sent a PR to fix this, thought there was no such expected error, however it then turned out that the underlying StakeMath library catches this case and reverts accordingly. Here's the PR: #129

As a result of this @gravityblast mentioned that these errors should not come from the library (which aligns with my initial feeling).

So we agreed on revisiting this.

0x-r4bbit added a commit that referenced this issue Feb 20, 2025
ones

There are a bunch of error codes that are either similar to other ones
or not used at all.

This commit moves them to the interface and removes the ones that aren't
used anymore. Part of the reason we have so many unused errors becuase
they had been "reintroduced" in `StakeMath`, which we'll revisit as
well as described in #130
0x-r4bbit added a commit that referenced this issue Feb 20, 2025
ones

There are a bunch of error codes that are either similar to other ones
or not used at all.

This commit moves them to the interface and removes the ones that aren't
used anymore. Part of the reason we have so many unused errors becuase
they had been "reintroduced" in `StakeMath`, which we'll revisit as
well as described in #130
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant