Temporary param update #352
Conversation
actors/abi/epoch.go
Outdated
|
|
||
| // The duration of a chain epoch. | ||
| // This is used for deriving epoch-denominated periods that are more naturally expressed in clock time. | ||
| const EpochDurationSeconds = 25 |
There was a problem hiding this comment.
is this the variable we should use instead of BlockTimeSeconds in reward_state.go soon?
There was a problem hiding this comment.
I believe so, will let @anorth confirm
There was a problem hiding this comment.
can we make some of these vars? that way we can change them live in the implementations for testing things.
There was a problem hiding this comment.
it would pain me, but in lieu of taking time to make them properly configurable, I'm on board.
There was a problem hiding this comment.
will let you guys make the change here in a different PR
There was a problem hiding this comment.
I tried, but the change must flow on to all the other constants that are derived from this value, which need to read the live-changed value rather than the initial one set here, and thus be functions. I opted not to force this on us so close to testnet-2.
actors/abi/epoch.go
Outdated
|
|
||
| // The duration of a chain epoch. | ||
| // This is used for deriving epoch-denominated periods that are more naturally expressed in clock time. | ||
| const EpochDurationSeconds = 25 |
actors/builtin/market/policy.go
Outdated
| @@ -7,7 +7,7 @@ const DealUpdatesInterval = 100 | |||
|
|
|||
| // Bounds (inclusive) on deal duration | |||
| func dealDurationBounds(size abi.PaddedPieceSize) (min abi.ChainEpoch, max abi.ChainEpoch) { | |||
| return abi.ChainEpoch(0), abi.ChainEpoch(10000) // PARAM_FINISH | |||
| return abi.ChainEpoch(0), abi.ChainEpoch(abi.SecondsInYear / abi.EpochDurationSeconds) // min 0, max 1y, PARAM_FINISH | |||
There was a problem hiding this comment.
Why 1 year? Why is there a max? Why not a millenium? If there's some concrete justification please write it here in a comment, otherwise use a "never going to happen" large number.
There was a problem hiding this comment.
we are expecting to upgrade PoRep fairly often anyways. There is no reasons for an arbitrarily long deals and we are looking to add feature to extend deal duration post mainnet. Lastly, maximum deal duration of a year for testnet seems reasonable.
There was a problem hiding this comment.
I think it's more a question of what the number "means" -- I feel like the only good numbers for placeholders are 0 and infinity (regardless of whether our schedule means something like 1 year is "practically infinity").
There was a problem hiding this comment.
It’s not just a placeholder, it’s an assumption we have been making for a while in cryptoecon that we only consider sector lifetimes up to 1 year. After 1 year it may become rational to terminate (probably not, but I‘d want to think about it more if we want to increase this bound).
There was a problem hiding this comment.
Ok, I will document that assumption in the code.
Note that this deal bound does not bound sector lifetimes, though.
actors/builtin/miner/policy.go
Outdated
| Quantization: SecondsInDay / EpochDurationSeconds, // 1 day, PARAM_FINISH | ||
| InitialDelay: abi.ChainEpoch(7 * abi.SecondsInDay / abi.EpochDurationSeconds), // 1 week for testnet, PARAM_FINISH | ||
| VestPeriod: abi.ChainEpoch(7 * abi.SecondsInDay / abi.EpochDurationSeconds), // 1 week for testnet, PARAM_FINISH | ||
| StepDuration: abi.ChainEpoch(12 * abi.SecondsInHour / abi.EpochDurationSeconds), // 12 hours for testnet, PARAM_FINISH |
There was a problem hiding this comment.
what happens if EpochDurationSeconds doesnt evenly divide secondsInHour?
There was a problem hiding this comment.
it will be truncated which i think it's fine, we will have to add test cases here to make sure
There was a problem hiding this comment.
As long as the remainder 12*SecondsInHour % EpochDurationSeconds is less than, say, 1 percent of 12*SecondsInHour, it’s not so bad. But it would be really nice if the epoch duration is a divisor of 3600. There are so many: 1 2 3 4 5 6 8 9 10 12 15 16 18 20 24 25 30 36 40 45 48 50 60 72 75 80 90 100...
|
I am going to make some cosmetic changes and then approve and land this. |
Updating parameters for unincentivized testnet, we expect further updates before testnet incentives and mainnet launch and even after mainnet.