[Merged by Bors] - Remove dependency on activation/wire from sql/identities #6330
Conversation
fasmat
requested review from
dshulyak,
poszu,
ivan4th and
acud
as code owners
fasmat
force-pushed
the
no-wire-dependency
branch
from
01a74c3
to
7a38fc7
Compare
malfeasance/handler_test.go
Outdated
| var blob sql.Blob | ||
| require.NoError(t, identities.LoadMalfeasanceBlob(context.Background(), h.db, nodeID.Bytes(), &blob)) | ||
| malProof := &wire.MalfeasanceProof{} | ||
| codec.MustDecode(blob.Bytes, malProof) |
There was a problem hiding this comment.
This is repeated so often, maybe it deserves a helper in malfeasance package?
func LoadMalfeasanceProof(db sql.Executor, id types.NodeID) (*mwire.MalfeasanceProof, error) { ... }There was a problem hiding this comment.
I thought about that as well. The reason I didn't implemented a function that returns a wire.MalfeasanceProof from the DB directly is because on the one hand this function would only used in tests and arguably it the tests instead should assert that an identity has been marked as malfeasant instead of fetching the proof from the DB and asserting that a specific proof was stored (separation of concerns).
Wdyt about simplifying the relevant tests to only check for malfeasance instead of asserting that a specific proof was stored?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6330 +/- ##
=======================================
Coverage 81.8% 81.8%
=======================================
Files 312 312
Lines 34623 34606 -17
=======================================
- Hits 28334 28328 -6
+ Misses 4458 4446 -12
- Partials 1831 1832 +1 ☔ View full report in Codecov by Sentry. |
|
bors merge |
## Motivation The `sql/identities` package depends on `activation/wire`. `sql` packages should not import `wire` packages, rather should be oblivious to specific encodings of wire types. This PR removes this dependency.
|
Build failed:
|
|
Flaky test: #6262 bors merge |
## Motivation The `sql/identities` package depends on `activation/wire`. `sql` packages should not import `wire` packages, rather should be oblivious to specific encodings of wire types. This PR removes this dependency.
|
Build failed: |
|
grpc errors bors merge |
## Motivation The `sql/identities` package depends on `activation/wire`. `sql` packages should not import `wire` packages, rather should be oblivious to specific encodings of wire types. This PR removes this dependency.
|
Pull request successfully merged into develop. Build succeeded: |
spacemesh-bors
bot
changed the title
Motivation
The
sql/identitiespackage depends onactivation/wire.sqlpackages should not importwirepackages, rather should be oblivious to specific encodings of wire types.This PR removes this dependency.
Description
With an upcoming extension of malfeasance proofs I want to use the
sql/identitiespackage as an import inactivation/wire. Without this change this won't be possible. Packages needing malfeasance proofs should call thesql/identitiespackage to get the raw bytes and decode the returned bytes themselves if needed.Test Plan
Existing tests were adjusted and checked to still pass
TODO