10016 StakingAccountQueries #10069

turadg · 2024-09-11T18:29:29Z

Description

Implements the StakingAccountQueries. Except for getRedelegation because there is no query that's guaranteed to return one. It's a strict subset of what's returned by getRedelegations. If there are ever use cases in which that payload is too large we can optimize then.

This also massages the return types in cosmos-api.ts to more closely match what is returned by the Cosmos query endpoints. (No longer dropping data.)

Security Considerations

The queries are read-only. Like any query there is a risk of resource exhaustion. These aren't rate limited but since they run in a vat the malicious code would exhaust itself. The risks of exhausting the RPC server are no greater than any off-chain query.

Scaling Considerations

May benefit from some load testing in production-ish environment to see what the service can handle.

Documentation Considerations

The interface is already documented. This implements it so nothing changes (other than the return types, as mentioned above)

Testing Considerations

New tests

Upgrade Considerations

No vats yet use this code.

cloudflare-workers-and-pages · 2024-09-11T18:29:31Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`c006436`
Status:	✅ Deploy successful!
Preview URL:	https://b64abe8d.agoric-sdk.pages.dev
Branch Preview URL:	https://10016-stakingaccountqueries.agoric-sdk.pages.dev

View logs

turadg · 2024-09-17T00:09:45Z

packages/orchestration/src/examples/unbond.flows.js

+  await osmoAccount.undelegate(
+    delegations.map(d => ({
+      validator: {
+        chainId: 'osmosis-1',
+        value: /** @type {`${string}valoper${string}`} */ (
+          d.delegation.validatorAddress
+        ),
+        encoding: 'bech32',
+      },
+      amount: { denom: 'uosmo', value: BigInt(d.balance.amount) },
+    })),
+  );


the mismatch between the return of getDelegations and the param of undelegate is clunky. I'll see about cleaning that up before calling this R4R.

Other feedback welcomed though

In 6420537 we moved away from using the Delegation: { validatorAddress, delegatorAddress, shares } type with undelegate to better align with the .delegate() and .redelegate().

I think this was the correct decision to better align the API, so I would support seeing us massage to getDelegations: () => { validator: CosmosValidatorAddress, amount: DenomAmount[] } if that's what you're aiming for.

0xpatrickdev

Looks good, only critical feedback item is ensure toDenomAmount doesn't throw when we give it a decimal.

I also assume your view is that LocalOrchAccount queries are out of scope for this task. I'm fine with this as we don't have any stories that require it, but think we should make a note or task item on #10048

0xpatrickdev · 2024-09-17T15:18:34Z

packages/orchestration/src/examples/unbond.flows.js

+  await osmoAccount.undelegate(
+    delegations.map(d => ({
+      validator: {
+        chainId: 'osmosis-1',
+        value: /** @type {`${string}valoper${string}`} */ (
+          d.delegation.validatorAddress
+        ),
+        encoding: 'bech32',
+      },
+      amount: { denom: 'uosmo', value: BigInt(d.balance.amount) },
+    })),
+  );


In 6420537 we moved away from using the Delegation: { validatorAddress, delegatorAddress, shares } type with undelegate to better align with the .delegate() and .redelegate().

I think this was the correct decision to better align the API, so I would support seeing us massage to getDelegations: () => { validator: CosmosValidatorAddress, amount: DenomAmount[] } if that's what you're aiming for.

0xpatrickdev · 2024-09-17T15:27:19Z

packages/orchestration/src/exos/cosmos-orchestration-account.js

+          return harden({
+            rewards: rewards.map(reward => ({
+              validatorAddress: reward.validatorAddress,
+              reward: reward.reward.map(toDenomAmount),


Should we need to add logic to toDenomAmount to round down to the nearest whole integer? I see that this query returns DecCoin instead of the typical Coin.

Alternatively, accounting for decimals with ratios is an option, but I'm not sure it's worth the cost here.

Good catch! I've added tests and helpers for this

0xpatrickdev · 2024-09-17T15:29:43Z

packages/orchestration/src/cosmos-api.ts

@@ -97,6 +100,12 @@ export type CosmosChainInfo = Readonly<{
  stakingTokens?: Readonly<Array<{ denom: string }>>;
 }>;

+/** @see {QueryDelegationTotalRewardsResponse} */
+export interface OrchestrationQueryDelegationTotalRewardsResponse {
+  rewards: { validatorAddress: string; reward: DenomAmount[] }[];


nb: Consider

Suggested change

rewards: { validatorAddress: string; reward: DenomAmount[] }[];

rewards: { validator: CosmosValidatorAddress; amount: DenomAmount[] }[];

for consistency, although we don't need this response for withdrawReward or withdrawRewards

0xpatrickdev · 2024-09-17T15:33:11Z

packages/orchestration/src/cosmos-api.ts


  /**
   * Get the pending rewards for the account.
   * @returns the amounts of the account's rewards pending from all validators
   */
-  getRewards: () => Promise<DenomAmount[]>;
+  getRewards: () => Promise<OrchestrationQueryDelegationTotalRewardsResponse>;


I like what OrchestrationQueryDelegationTotalRewardsResponse computes to, but consider a shorter name. I recognize something like DelegationTotalRewards or DelegationRewards might look too much like a cosmic-proto type, but I would prefer the brevity at this expense.

gonna try CosmosGetRewardsResponse. Pattern being: "Cosmos" to convey that it's part of cosmos-api and it's clearly not part one of the protobuf types because they don't need to say that. Middle is the method name ("getRewards") and "Response" is because it's the return value.

hmm, all the queries start with get and Response implies a query so maybe CosmosRewardsResponse.

0xpatrickdev · 2024-09-17T15:41:05Z

packages/orchestration/test/examples/unbond.contract.test.ts

+            balance: { denom: 'uatom', amount: '1000000' },
+          },
+        ],
+        pagination: { nextKey: new Uint8Array(), total: 1n },


Do we handle/support pagination? Not sure it's worth the cost atm, but curious what it looks like. Maybe it's as simple as exposing a nextKey as an optional param in getDelegations() and the other interfaces that support it.

0xpatrickdev · 2024-09-17T21:46:19Z

packages/orchestration/tools/protobuf-decoder.js

@@ -0,0 +1,153 @@
+/* eslint-env node */


Nice! (entire file 🙂)

There's no query that returns a single Redelegation

turadg force-pushed the 10016-StakingAccountQueries branch from 81a8fec to cc949e6 Compare September 16, 2024 22:00

turadg requested review from dckc and 0xpatrickdev September 16, 2024 22:08

turadg marked this pull request as ready for review September 16, 2024 22:08

turadg marked this pull request as draft September 17, 2024 00:08

turadg commented Sep 17, 2024

View reviewed changes

0xpatrickdev reviewed Sep 17, 2024

View reviewed changes

turadg requested a review from 0xpatrickdev September 17, 2024 18:51

turadg marked this pull request as ready for review September 17, 2024 19:00

0xpatrickdev approved these changes Sep 17, 2024

View reviewed changes

turadg added 8 commits September 17, 2024 14:50

refactor: extract stakingAccountActionsMethods

fdafe96

fix: remove getRedelegation

f39c2cf

There's no query that returns a single Redelegation

feat: toCosmosValidatorAddress

43ba807

feat(network-fakes): improve help

5834ec5

feat: toTruncatedDenomAmount

a218a19

feat: toCosmosDelegationResponse

8a6f9a5

feat: StakingAccountQueries

a4a7aa9

test(examples): undelegate

c006436

turadg force-pushed the 10016-StakingAccountQueries branch from 886fb0a to c006436 Compare September 17, 2024 21:58

turadg added the automerge:rebase Automatically rebase updates, then merge label Sep 17, 2024

mergify bot merged commit f291362 into master Sep 17, 2024
90 checks passed

mergify bot deleted the 10016-StakingAccountQueries branch September 17, 2024 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

10016 StakingAccountQueries #10069

10016 StakingAccountQueries #10069

turadg commented Sep 11, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Sep 11, 2024 •

edited

Loading

turadg Sep 17, 2024

0xpatrickdev Sep 17, 2024

0xpatrickdev left a comment

0xpatrickdev Sep 17, 2024

0xpatrickdev Sep 17, 2024

turadg Sep 17, 2024

0xpatrickdev Sep 17, 2024

0xpatrickdev Sep 17, 2024

turadg Sep 17, 2024

turadg Sep 17, 2024

0xpatrickdev Sep 17, 2024

0xpatrickdev Sep 17, 2024 •

edited

Loading

	rewards: { validatorAddress: string; reward: DenomAmount[] }[];
	rewards: { validator: CosmosValidatorAddress; amount: DenomAmount[] }[];

10016 StakingAccountQueries #10069

10016 StakingAccountQueries #10069

Conversation

turadg commented Sep 11, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Sep 11, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

turadg commented Sep 11, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Sep 11, 2024 •

edited

Loading

0xpatrickdev Sep 17, 2024 •

edited

Loading