Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Limit longevity of im-online heartbeats. #4011

Merged
gavofyork merged 3 commits into from
Nov 5, 2019
Merged

Limit longevity of im-online heartbeats. #4011

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e72730b
Limit longevity of im-online heartbeats.
tomusdrw Nov 4, 2019
fe89c94
Unused import.
tomusdrw Nov 4, 2019
b603195
Use parameter for session duration.
tomusdrw Nov 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Limit longevity of im-online heartbeats.
  • Loading branch information
@tomusdrw
tomusdrw committed Nov 4, 2019
commit e72730bd67e37d5a6e8498a972fdcaaa6c262080
2 changes: 1 addition & 1 deletion srml/im-online/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ impl<T: Trait> support::unsigned::ValidateUnsigned for Module<T> {
priority: TransactionPriority::max_value(),
requires: vec![],
provides: vec![(current_session, authority_id).encode()],
longevity: TransactionLongevity::max_value(),
longevity: 128,
Copy link
Member

@bkchr bkchr Nov 4, 2019

Choose a reason for hiding this comment

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

Hmm, why not make this parameter configurable? Or make it proportional to the length of a session?

Whatever, but I would not hardcode this parameter.

Copy link
Contributor Author

@tomusdrw tomusdrw Nov 4, 2019

Choose a reason for hiding this comment

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

Fair enough. We can make a parameter for this I suppose, the actual session length is not easy to retrieve, as it's controlled by ShouldEndSession handler, so for instance for Kusama it's controlled by babe, but it's fine to just pass it as parameter.

Copy link
Member

@bkchr bkchr Nov 4, 2019

Choose a reason for hiding this comment

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

AFAIK the session length is fixed. You are right that we call into Babe to know if a session should end, but Babe internally does not change the length or whatever.
Even if the session length is not 100% correct, it will not make anything worse in this use case here. The transaction would just be valid for a little bit more time than it should be.

propagate: true,
})
} else {
Expand Down