Add extra support for more getters for the Validator class #344
Conversation
src/coinbase/validator.ts
Outdated
| */ | ||
| public toString(): string { | ||
| return `Id: ${this.getValidatorId()} Status: ${this.getStatus()}`; | ||
| return `Validator { Id: ${this.getValidatorId()}, Status: ${this.getStatus()}, Exit Epoch: ${this.getExitEpoch()}, Network ID: ${this.getNetworkId()}, Asset ID: ${this.getAssetId()}, Index: ${this.getIndex()}, Public Key: ${this.getPublicKey()}, Slashed: ${this.isSlashed()}, Withdrawable Epoch: ${this.getWithdrawableEpoch()}, Withdrawal Address: ${this.getWithdrawalAddress()}, Effective Balance: { Amount: ${this.getEffectiveBalance()?.amount}, AssetID: { ${this.getEffectiveBalance()?.asset.asset_id}, Decimals: ${this.getEffectiveBalance()?.asset.decimals}, NetworkID: ${this.getEffectiveBalance()?.asset.network_id} } }, Balance: { Amount ${this.getBalance()?.amount}, Asset: { AssetID: ${this.getBalance()?.asset.asset_id}, Decimals: ${this.getBalance()?.asset.decimals}, NetworkID: ${this.getBalance()?.asset.network_id} } }, Activation Epoch: ${this.getActivationEpoch()} }`; |
There was a problem hiding this comment.
I think toString should still be the simpler one. Primarily used of logging etc. This will be too much to log.
There was a problem hiding this comment.
Yeah I see. Truly wasn't sure what to do here. I'm fine with simplifying this.
Are you suggesting just printing out the top level data values? or what subset specifically would be important?
There was a problem hiding this comment.
Just id and status like before and updating the comment acccordingly.
| * | ||
| * @returns The activation epoch as a string. | ||
| */ | ||
| public getActivationEpoch(): string { |
There was a problem hiding this comment.
the problem with this is - these are very ethereum specific.
In future if we say support sol validators, do we still want user to do validator. getActivationEpoch() ?
Do we want to support a helper function of this format instead validator. getEthereumDetails().getActivationEpoch() ?
There was a problem hiding this comment.
I'd be more in favor of revisiting this when we cross that bridge. i.e. I don't see us exposing Solana validator staking anytime soon.
| @@ -1,6 +1,11 @@ | |||
| import { Coinbase } from "./coinbase"; | |||
| import { Validator as ValidatorModel, ValidatorStatus as APIValidatorStatus } from "../client/api"; | |||
| import { | |||
There was a problem hiding this comment.
maybe some unit tests for these getters ?
What changed? Why?
Adding support for more getters to expose the data behind the Validator object.
Example
validator.toJSON()Qualified Impact