feat: changes to support cost models while updating Alonzo protocols through genesis config #984
Conversation
Signed-off-by: Jenita <[email protected]>
There was a problem hiding this comment.
While this approach will technically work for decoding the Alonzo genesis in most networks, we'll want to be more explicit with the mapping of these keys to numeric index. This will allow us to convert back and forth between the formats, as well as do things like validate the inputs. Each of the 3 Plutus versions will have their own mapping.
We also need to be able to support the simple list format as seen in the devnet genesis config:
https://github.com/blinklabs-io/docker-cardano-configs/blob/main/config/devnet/alonzo-genesis.json
ledger/alonzo/pparams.go
Outdated
| for lang, model := range genesis.CostModels { | ||
| var langKey uint | ||
| switch lang { | ||
| case "plutus:v1": |
There was a problem hiding this comment.
This is PlutusV1 in the JSON
ledger/alonzo/pparams.go
Outdated
| switch lang { | ||
| case "plutus:v1": | ||
| langKey = 0 | ||
| case "plutus:v2": |
There was a problem hiding this comment.
This is PlutusV2 in the JSON
ledger/alonzo/pparams.go
Outdated
| langKey = 0 | ||
| case "plutus:v2": | ||
| langKey = 1 | ||
| default: |
There was a problem hiding this comment.
It's also possible to have a PlutusV3
We have a separate issue for that one. |
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
ledger/alonzo/pparams.go
Outdated
| @@ -44,7 +60,7 @@ type AlonzoProtocolParameters struct { | |||
| MinUtxoValue uint | |||
| MinPoolCost uint64 | |||
| AdaPerUtxoByte uint64 | |||
| CostModels map[uint][]int64 | |||
| CostModels map[PlutusVersion]*CostModel | |||
There was a problem hiding this comment.
This is (probably) going to break CBOR decoding of Alonzo protocol params. The previous struct field definition matches the structure of the CBOR data.
ledger/alonzo/pparams.go
Outdated
| ProtocolMinor: prevPParams.ProtocolMinor, | ||
| MinUtxoValue: prevPParams.MinUtxoValue, | ||
| } | ||
| } |
There was a problem hiding this comment.
Why did you remove this function? We use this in dingo
There was a problem hiding this comment.
Apologies, I removed this function unintentionally while resolving the conflicts. fixing it
ledger/alonzo/pparams.go
Outdated
| case "plutus:v2", "plutusv2": | ||
| return PlutusV2, true | ||
| case "plutus:v3", "plutusv3": | ||
| return PlutusV3, true |
There was a problem hiding this comment.
Where are any of these forms used?
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
| } | ||
| intVal = int64(floatVal) | ||
| } | ||
| versionMap[k] = int(intVal) |
There was a problem hiding this comment.
Do we encounter all 3 of these types when parsing example Alonzo genesis files, or is this just covering our bases?
| @@ -29,7 +30,7 @@ type AlonzoGenesis struct { | |||
| ExecutionPrices AlonzoGenesisExecutionPrices `json:"executionPrices"` | |||
| MaxTxExUnits AlonzoGenesisExUnits `json:"maxTxExUnits"` | |||
| MaxBlockExUnits AlonzoGenesisExUnits `json:"maxBlockExUnits"` | |||
| CostModels map[string]map[string]int `json:"costModels"` | |||
| CostModels map[string]interface{} `json:"costModels"` | |||
There was a problem hiding this comment.
This field type is going to force us to do something like foo.CostModels.(map[string]int)["foo"] every time that we access something from it, which isn't ideal. What we probably want is a separate CostModel type with a custom UnmarshalJSON() function that works similarly to NormalizeCostModels() below.
| if genesis.ExecutionPrices.Mem != nil && | ||
| genesis.ExecutionPrices.Steps != nil { | ||
|
|
||
| if genesis.ExecutionPrices.Mem != nil && genesis.ExecutionPrices.Steps != nil { |
There was a problem hiding this comment.
This is going to get split back to 2 lines the next time that we run golines on this repo
| if _, err := fmt.Sscanf(paramName, "param%d", &index); err == nil && index > maxIndex { | ||
| maxIndex = index | ||
| } | ||
| } |
There was a problem hiding this comment.
This seems to be expecting a structure like this, but I'm not seeing where that would come from outside of the tests. It seems that it would be easier to use []int64 or similar.
{ "param0": 123, "param1": 234, "param2": 345 }
| for i, val := range v { | ||
| if intVal, ok := val.(float64); ok { | ||
| values[i] = int64(intVal) | ||
| } |
There was a problem hiding this comment.
Will this only ever contain float64 values? It looks like anything else would be ignored
Closes #852
Closes #963