Remove getters #492
Remove getters #492
Conversation
* Filter shares for slot `CommitteeRunner` based on validators that have duty for that slot. * Filter duty and create share map according to owned validators * Add test: start duty with no shares for duty's validators * Add test: happy flow for committee with fraction of duty's validators * Generate JSON tests * Apply suggestions --------- Co-authored-by: MatheusFranco99 <[email protected]>
* Update go1.20 to go1.22 * Update go.sum with mod tidy
* Update dependencies * Fix lint issue * Generate JSON tests to trigger actions * Update fastssz * Generate JSON tests and align ssz error * Revert go-eth2-client version change * Revert fastssz upgrade * Generate SSZ and JSON tests
* Solve potential file inclusion via variable * Fix file permission (0644 to 0600) * Add nosec comment for PRNG (pseudo-random number generator) used for testing * Fix lint issue on nil check in []byte type * Update permission from 0444 to 0600 * Update 0444 to 0400
* Remove nolint comment and export timeout variables * Drop unnecessary nolint * Add comment * Fix lint issue
* Add share length validation in runner construction * Align to error handling in runners constructions * Add validation to committee runner * Add runner construction tests * Refactor runner construction in testingutil to deal with creation errors * Generate JSON tests * Fix lint issue * Fix comments
* Remove redundant validation * Align error string
* Sort signers in decided message * Add test for sorted signers in decided msg * Generate JSON tests * Fix lint issue
* Stop processing consensus messages after instance is decided * Align error in qbft tests * Align errors in ssv tests * Generate JSON tests * Fix lint issue
* Remove leftover err check * Align argument variable name to type
qbft/json_testutils.go
Outdated
| c.OperatorSigner = aux.OperatorSigner | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
how come this is added here?
There was a problem hiding this comment.
Before, config was private (first letter lowercase), so it didn't appear in JSONs. With GetConfig being dropped and configConfig switching to a public attribute, it started appearing in JSONs. However, it can't be unmarshalled due to its function types. So, it was necessary to hide the config in the custom marshalling.
Note: in tests, we fill the Config attribute with the default testing values.
Right @alan-ssvlabs ?
There was a problem hiding this comment.
Totally correct, thanks Matheus
There was a problem hiding this comment.
It is better to add json:"-" next to config to make serialization ignore this field and remove the extra code
There was a problem hiding this comment.
Didn't know this can work, updated, thanks!
There was a problem hiding this comment.
Didn't know this can work, updated, thanks!
| @@ -125,7 +125,7 @@ Content hash is used: | |||
|
|
|||
| ```go | |||
| func MsgID(msg) string { | |||
| return string(sha256(msg.GetData())[:20]) | |||
| return string(sha256(msg.Data)[:20]) | |||
GalRogozinski
force-pushed
the
dev
branch
2 times, most recently
from
b5bd1d3 to
bf0ff16
Compare
Fixes #465, this PR removes getter functions that do not require computations.