feat(shelley): created genesis pools from shelley genesis #1054
Conversation
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
ledger/shelley/genesis.go
Outdated
| } | ||
|
|
||
| // GetInitialPools returns all initial stake pools with their delegators | ||
| func (g *ShelleyGenesis) GetInitialPools() (map[string]GenesisPool, map[string][]string, error) { |
There was a problem hiding this comment.
Drop the Get prefix on the function. It's not idiomatic Go
ledger/shelley/genesis.go
Outdated
| } | ||
|
|
||
| // GetPoolById returns a specific pool by its ID along with its delegators | ||
| func (g *ShelleyGenesis) GetPoolById(poolId string) (*GenesisPool, []string, error) { |
There was a problem hiding this comment.
Drop the Get prefix on the function. It's not idiomatic Go
ledger/shelley/genesis.go
Outdated
| } | ||
|
|
||
| // GetInitialPools returns all initial stake pools with their delegators | ||
| func (g *ShelleyGenesis) GetInitialPools() (map[string]GenesisPool, map[string][]string, error) { |
There was a problem hiding this comment.
We should return native ledger types here like we do above for GenesisUtxos(). At minimum, we should use common.Address where appropriate.
ledger/shelley/genesis.go
Outdated
| } | ||
|
|
||
| // GetPoolById returns a specific pool by its ID along with its delegators | ||
| func (g *ShelleyGenesis) GetPoolById(poolId string) (*GenesisPool, []string, error) { |
There was a problem hiding this comment.
This should return []common.Address instead of []string
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]>
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
ledger/common/certs.go
Outdated
| switch v := marginValue.(type) { | ||
| case float64: | ||
| p.Margin.Rat = new(big.Rat).SetFloat64(v) | ||
| case []interface{}: |
There was a problem hiding this comment.
Have you actually seen this in example JSON? If so, we should add support for decoding that format to GenesisRat.
ledger/common/certs.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("failed to decode reward account key hash: %w", err) | ||
| } | ||
| if len(hashBytes) != 28 { |
There was a problem hiding this comment.
Use the AddressHashSize constant instead of a magic number
ledger/common/certs.go
Outdated
| if len(tmp.RewardAccount) > 0 { | ||
| type credential struct { | ||
| KeyHash string `json:"key hash"` | ||
| } |
There was a problem hiding this comment.
This type can be nested inline in the next one, since it's never used separately, or outside of this code block
ledger/common/certs.go
Outdated
| return fmt.Errorf("invalid key hash length: expected 28, got %d", len(hashBytes)) | ||
| } | ||
| var hash Blake2b224 | ||
| copy(hash[:], hashBytes) |
There was a problem hiding this comment.
You can use common.NewBlake2b224(hashBytes) here, which does the copy() for you.
ledger/common/certs.go
Outdated
|
|
||
| switch v := marginValue.(type) { | ||
| case float64: | ||
| p.Margin.Rat = new(big.Rat).SetFloat64(v) |
There was a problem hiding this comment.
We want to avoid using a float to represent any of our ratios anywhere along the way, since a loss of precision could result in different outcomes from calculations using those values.
ledger/shelley/genesis.go
Outdated
| return nil, nil, errors.New("failed to decode stake key") | ||
| } | ||
|
|
||
| stakeAddrBytes := append([]byte{0xE1}, stakeKeyBytes...) |
There was a problem hiding this comment.
The 0xE1 here probably isn't correct. This represents a script address on mainnet, which is not going to be correct everywhere
There was a problem hiding this comment.
have addressed the comments, could you please review again ?
Signed-off-by: Chris Gianelloni <[email protected]>
Signed-off-by: Jenita <[email protected]>
ledger/shelley/genesis.go
Outdated
| return headers.Script, nil | ||
| } | ||
| return headers.Base, nil | ||
| } |
There was a problem hiding this comment.
You can replace all of this with something like:
newAddr := common.NewAddressFromParts(
common.AddressTypeNoneKey,
common.AddressNetworkMainnet,
nil,
stakeKeyHashBytes,
)
For a script stake address, use common.AddressTypeNoneScript as the first arg.
You will probably need to also modify NewAddressFromParts() to allow a zero-length payment address part to do this.
Signed-off-by: Jenita <[email protected]>
ledger/common/address.go
Outdated
| addrBytes = append(addrBytes, stakingAddr...) | ||
| } | ||
|
|
||
| return NewAddressFromBytes(addrBytes) |
There was a problem hiding this comment.
There's no need to do the round-trip through byte. Use the method that we already had to build the new Address and return it.
ledger/common/address.go
Outdated
| } | ||
|
|
||
| if addrType == AddressTypeNoneScript && networkId == AddressNetworkTestnet { | ||
| header := byte(0xF1) |
There was a problem hiding this comment.
We don't want to hard-code magic values like this
ledger/common/address.go
Outdated
|
|
||
| header := addrType<<4 | networkId | ||
| addrBytes := append([]byte{header}, stakingAddr...) | ||
| return NewAddressFromBytes(addrBytes) |
There was a problem hiding this comment.
Use the method that we already had for building a new Address rather than round-tripping it through bytes
Signed-off-by: Jenita <[email protected]>
Signed-off-by: Jenita <[email protected]>
ledger/common/address.go
Outdated
|
|
||
| if a.addressType == AddressTypeNoneScript && a.networkId == AddressNetworkTestnet { | ||
| header = (AddressTypeNoneScript << 4) | 1 | ||
| } |
There was a problem hiding this comment.
I'm not sure what's happening here. This seems like a very specific circumstance that we could probably account for in other ways
There was a problem hiding this comment.
eliminated this , it was for a test only.
Signed-off-by: Jenita <[email protected]>
Closes #1042