-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
List staking balances #134
Conversation
🟡 Heimdall Review Status
|
a8490d4
to
1f4507e
Compare
/** | ||
* Returns a list of StakingBalances for the provided network, asset, and address. | ||
* | ||
* @param networkId - The network ID. | ||
* @param assetId - The asset ID. | ||
* @param address - The address ID. | ||
* @param startTime - The start time. | ||
* @param endTime - The end time. | ||
* @returns The staking balances. | ||
*/ | ||
public static async list( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a method on the Address
class (ref) instead of static method here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have an equivalent for listing staking rewards , I think we need both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. That pattern is fine. Can you please add the Address
class change to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added, wondering if I can test the change to Address, currently only unit tests added to all related files including Wallet
, External Address
& Wallet Address
hardcoded the address in the list
and verify the call from address functions well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Thanks! You can also call this method on an ExternalAddress
subclass instance if you want to test on a specific address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, that works as well
const addr = new ExternalAddress(Coinbase.networks.EthereumMainnet, "0x80000001677f23a227dfed6f61b132d114be83b8ad0aa5f3c5d1d77e6ee0bf5f73b0af750cc34e8f2dae73c21dc36f4a")
addr.historicalStakingBalances(Coinbase.assets.Eth, "2024-07-01T00:00:00Z", "2024-07-03T00:00:00Z").then(balances => {
balances.forEach(balance => {console.log(balance)});
} ).catch(error => {
console.log(error);
});
StakingBalance {
model: {
address: '0x80000001677f23a227dfed6f61b132d114be83b8ad0aa5f3c5d1d77e6ee0bf5f73b0af750cc34e8f2dae73c21dc36f4a',
bonded_stake: { amount: '32', asset: [Object] },
date: '2024-07-02',
participant_type: 'VALIDATOR',
unbonded_balance: { amount: '5.479839548', asset: [Object] }
}
}
toString()
StakingBalance { date: '2024-07-02T00:00:00.000Z' address: '0x80000001677f23a227dfed6f61b132d114be83b8ad0aa5f3c5d1d77e6ee0bf5f73b0af750cc34e8f2dae73c21dc36f4a' bondedStake: 'Balance { amount: '32' asset: 'Asset{ networkId: ethereum-mainnet, assetId: eth, contractAddress: undefined, decimals: 18 }' }' unbondedBalance: 'Balance { amount: '5.479839548' asset: 'Asset{ networkId: ethereum-mainnet, assetId: eth, contractAddress: undefined, decimals: 18 }' }' participantType: 'VALIDATOR' }
23a2b0a
to
ab3337f
Compare
Please add an update to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please prefix all of the static method jest descriptions with #
Ex. describe("#staticMethodName", () => {...}
Otherwise LGTM!
jest.clearAllMocks(); | ||
}); | ||
|
||
describe(".list", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe(".list", () => { | |
describe("#list", () => { |
What changed? Why?
v0.0.16
StakingBalance
with static method to get historical staking balances from backendAddress\Wallet\External Address
Qualified Impact
Result
Model
ToString()