fix: heartbeat timestamp unit, transfer normalization, validator propagation, error handling - #46
Open
Alicepoltora wants to merge 1 commit into
Conversation
…agation, error handling 1. Fix heartbeat timestamp: seconds → milliseconds (setu-rpc/src/registration.rs) - Heartbeat used as_secs() while entire codebase uses milliseconds - Staleness checks were off by 1000×, causing false timeout/alive decisions 2. Fix transfer forwards unnormalized addresses (setu-validator/src/user_handler.rs) - Anti-replay precheck and signature used normalized (lowercase) addresses - But SubmitTransferRequest forwarded original-case addresses - Downstream TEE runtime computed different nonce marker → precheck bypass 3. Fix add_validator not propagated to consensus engine (setu-validator/src/consensus_integration.rs) - add_validator() updated local ValidatorSet but NOT the engine's copy - New validators were invisible to quorum/leader election - add_peer_validator() already did this correctly; add_validator() was missing it 4. Fix GenericHandler swallowing errors (crates/setu-network-anemo/src/generic_handler.rs) - Handler errors returned 200 OK with empty body - Callers could not distinguish errors from legitimate empty responses - Now returns error message in body with __HANDLER_ERROR__: prefix
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 4 bugs across the RPC, validator, and network layers:
1. Heartbeat timestamp: seconds instead of milliseconds
setu-rpc/src/registration.rs:208The heartbeat client used
as_secs()to produce the timestamp, but every other timestamp in the codebase uses milliseconds (discoverytimestamp_ms, state synclast_update_ms, config timeouts in_ms). Any staleness check was off by 1000×, causing false timeout or false alive decisions.Fix:
as_secs()→as_millis() as u642. Transfer forwards unnormalized addresses — anti-replay bypass
setu-validator/src/user_handler.rs:1144The anti-replay precheck and canonical signature used
normalized_from/normalized_to(lowercase), butSubmitTransferRequestforwarded the original-caserequest.from/request.to. If the downstream TEE runtime derives the nonce marker object from the unnormalized address, it computes a different marker than the one pre-checked, rendering the anti-replay defense ineffective.Fix: Forward
normalized_from/normalized_toinstead ofrequest.from/request.to3.
add_validatornot propagated to consensus enginesetu-validator/src/consensus_integration.rs:846add_validator()updated the localValidatorSetcopy but did NOT callself.engine.add_consensus_validator(info). This meant new validators added via this path were invisible to quorum calculation and leader election.add_peer_validator()(line 475) already did this correctly.Fix: Added
self.engine.add_consensus_validator(info).awaitafter the local update4. GenericHandler swallows errors — returns 200 OK with empty body
crates/setu-network-anemo/src/generic_handler.rs:112When a message handler returned
Err(e), the service wrapper logged a warning and returnedOk(Response::new(Bytes::new()))— a 200 OK with empty body. Callers had no way to distinguish an error from a legitimate empty response.Fix: Return the error message in the response body with a
__HANDLER_ERROR__:prefix so callers can detect and parse errors.Testing
cargo check -p setu-rpc -p setu-validator -p setu-network-anemo— passescargo test -p setu-rpc -p setu-network-anemo— all passcargo test -p consensus -p setu-types— 338/339 pass (1 pre-existing golden-file failure)Files changed
setu-rpc/src/registration.rs—as_secs()→as_millis() as u64setu-validator/src/user_handler.rs— forward normalized addressessetu-validator/src/consensus_integration.rs— propagate to enginecrates/setu-network-anemo/src/generic_handler.rs— encode error in response