search

helium / blockchain-core

Apache License 2.0
213 stars 85 forks source link

Add var to harmonize a GWs activity on hip17 interactivity blocks #1360

Closed andymck closed 2 years ago

andymck commented 2 years ago

Related PR: https://github.com/helium/sibyl/pull/51

After 19.1 was cut it was realised that there were two vars in use to determine a GW was active or inactive, these were:

poc_v4_target_challenge_age hip17_interactivity_blocks

This harmonises the activity determination to use the hip17 var ( which has a higher permitted threshold of 5000 compared to 1000 for the former )

An alternative approach was to add a completely new var to determine the number of blocks before a GW is marked as inactive and replace both the above vars. The only scenario in which I thought that justified was if the max value of 5000 is considered constrictive. If that is the case then I will rework this....

vihu commented 2 years ago

I think this generally looks okay, but I don't like that there are changes here which are potentially unrelated to the problem at hand. If possible can we limit the scope to just handle the challenge_age and harmonize... var? The GC stuff is already gated behind h3dex_gc_width which is currently not set on chain, unless we plan on activating that with the next release, we should probably do a separate PR to the limit scope of this one.

Regardless, stamping :heavy_check_mark:

vihu commented 2 years ago

I think this generally looks okay, but I don't like that there are changes here which are potentially unrelated to the problem at hand. If possible can we limit the scope to just handle the challenge_age and harmonize... var? The GC stuff is already gated behind h3dex_gc_width which is currently not set on chain, unless we plan on activating that with the next release, we should probably do a separate PR to the limit scope of this one.

Regardless, stamping :heavy_check_mark: