feat: redelegate, undelegate on StakingAccountKit #9331

dckc · 2024-05-07T14:56:07Z

closes #9069
closes #9070

Description

on StakingAccountHolder, add Redelegate, Undelegate on invitatationMakers and redelegate(), undelegate() on holder.

refactor to get the holder facet to be checked against the orch API:

factor StakingAccountActions out of BaseOrchestrationAccount

was DRAFT until:

undelegate() waits for the unbonding period
spec/types use of cosmos types such as Delegation is cleared up
PR template (description etc.) filled out
git history clean-up

Security Considerations

adds a TimerService capabilty to each staking account kit.

Scaling Considerations

E(timer).wakeAt() consumes resources in the timer vat for the ~21 day unbonding period.

Documentation Considerations

orch API change, already discussed with product:

chore: narrow return type for redelegate to Promise<void> (or: widen, depending on how you look at it)

Testing Considerations

unit test coverage is reasonable.

I'd like to do more end-to-end testing; should closing #9069 wait for that?

Upgrade Considerations

doesn't use vows. Do we have plans to do that later? Which issue?

cloudflare-pages · 2024-05-07T14:57:29Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`67211a0`
Status:	✅ Deploy successful!
Preview URL:	https://9d924dd5.agoric-sdk.pages.dev
Branch Preview URL:	https://dc-dist-helper.agoric-sdk.pages.dev

View logs

packages/orchestration/src/types.d.ts

packages/orchestration/test/test-withdraw-reward.js

packages/orchestration/src/exos/stakingAccountKit.js

dckc · 2024-05-07T15:40:41Z

packages/orchestration/src/exos/stakingAccountKit.js

+          const response = tryDecodeResponse(
+            result,
+            MsgDelegateResponse.fromProtoMsg,
+          );
+          mustMatch(harden(response), harden({})); // TODO: ignore response??


if we're not returning anything, is decoding the response worthwhile?

i don't think so. if it does start returning something this code can continue to ignore it

I agree with @turadg, but have wondered if it makes sense to make some of these (empty) response values constants for a quick sanity check - we know that the result string will always be Ei0KKy9jb3Ntb3Muc3Rha2luZy52MWJldGExLk1zZ0RlbGVnYXRlUmVzcG9uc2U= in the happy path.

packages/orchestration/src/exos/stakingAccountKit.js

dckc · 2024-05-07T15:43:07Z

packages/orchestration/src/exos/stakingAccountKit.js

+          trace('MsgBeginRedelegateResponse', response);
+          // TODO? return completionTime?


The spec says to return void. Do we really want to throw away completionTime?

product question

Since this is a Cosmos staking kit, I think we can return the Cosmos type. The higher level APIs can be more abstract. They might want to return void or a TimeRecord.

oh. based on discussion above, I just re-worked it to ignore the return value altogether.
I hope that's ok.

packages/orchestration/src/exos/stakingAccountKit.js

dckc · 2024-05-07T15:45:10Z

packages/orchestration/src/exos/stakingAccountKit.js

+  /** check holder facet against StakingAccountActions interface. FIXME: unused */
+  const typeCheck = () => {
+    /** @import {Guarded, Methods} from '@endo/exo'; */
+    /** @type {any} */
+    const arg = null;
+    /** @satisfies { StakingAccountActions } */
+    const kit = makeStakingAccountKit(arg, arg, arg, arg, arg).holder;
+  };


@turadg do you have any better idea than to keep this and silence the unused warnings?

dckc · 2024-05-07T15:46:00Z

packages/orchestration/src/typeGuards.js

@@ -18,3 +20,9 @@ export const Proto3Shape = {
 };

 export const CoinShape = { value: M.bigint(), denom: M.string() };


wait... Coin.value is a bigint? I see strings in a lot of places.

In the AnyJson version of Coin, bigints are cast to strings. It seems this can be removed as we are now returning ChainAmount. The interface guard for getBalance should return ChainAmountShape.

dckc · 2024-05-10T21:39:37Z

#9069 says:

This API should make use of the faux notification capability to return a promise that resolves when the redelegate (sic) is complete.

Since undelegate returns a completionTime, I didn't need any polling, so I didn't need anything special such as #9067.

I presume "redelegate" should be "undelegate" there.

turadg

Good stuff. @0xpatrickdev and I made some of the same API changes in #9356. Let's consider how to align them.

packages/orchestration/src/types.d.ts

turadg · 2024-05-10T23:01:37Z

packages/orchestration/src/examples/stakeAtom.contract.js

+        storageNode,
+        icqConnection,
+        // @ts-expect-error only for undelegate, which we do not use
+        timer: harden({}),


should it be optional then?

Seems worthwhile to include ChainTimerService in start-stakeAtom.js? makeAccount in StakeAtom returns a StakingAccountKit, so it does seem to be used.

oops... missed this one; tending to it now...

turadg · 2024-05-10T23:02:33Z

packages/orchestration/src/exos/stakingAccountKit.js

+         * @param {AmountArg} amount
+         * @returns {Coin}
+         */
+        amountToCoin(amount) {


good enough for now

turadg · 2024-05-10T23:04:03Z

packages/orchestration/src/exos/stakingAccountKit.js

+         * @param {CosmosValidatorAddress} dstValidator
+         * @param {AmountArg} amount
+         */
+        Redelegate(srcValidator, dstValidator, amount) {


since all these makers just map the methods from the holder, I wonder if we should have some method on the holder to create invitation makers given a zcf

I'm not clear on what you're suggesting.

Making another exoKit seems awkward.

turadg · 2024-05-10T23:05:07Z

packages/orchestration/src/exos/stakingAccountKit.js

+          const response = tryDecodeResponse(
+            result,
+            MsgDelegateResponse.fromProtoMsg,
+          );
+          mustMatch(harden(response), harden({})); // TODO: ignore response??


i don't think so. if it does start returning something this code can continue to ignore it

turadg · 2024-05-10T23:06:57Z

packages/orchestration/src/exos/stakingAccountKit.js

+          trace('MsgBeginRedelegateResponse', response);
+          // TODO? return completionTime?


Since this is a Cosmos staking kit, I think we can return the Cosmos type. The higher level APIs can be more abstract. They might want to return void or a TimeRecord.

0xpatrickdev

LGTM, as @turadg mentioned we might want to land #9356 first

0xpatrickdev · 2024-05-13T13:46:14Z

packages/orchestration/src/exos/stakingAccountKit.js


-          const result = await E(account).executeEncodedTx([
+          const result = await E(helper.owned()).executeEncodedTx([


0xpatrickdev · 2024-05-13T13:53:15Z

packages/orchestration/src/exos/stakingAccountKit.js

+          const response = tryDecodeResponse(
+            result,
+            MsgDelegateResponse.fromProtoMsg,
+          );
+          mustMatch(harden(response), harden({})); // TODO: ignore response??


I agree with @turadg, but have wondered if it makes sense to make some of these (empty) response values constants for a quick sanity check - we know that the result string will always be Ei0KKy9jb3Ntb3Muc3Rha2luZy52MWJldGExLk1zZ0RlbGVnYXRlUmVzcG9uc2U= in the happy path.

0xpatrickdev · 2024-05-13T14:07:16Z

packages/orchestration/src/exos/stakingAccountKit.js

+          );
+          trace('undelegate response', response);
+          const { completionTime } = response;
+          const endTime = BigInt(completionTime.valueOf() / 1000);


Is completionTime a Date object? Came here to see if there was any nuance in handling this in Jessie

Is completionTime a Date object?

yes. I suppose getTime() would be more clear than valueOf().

Came here to see if there was any nuance in handling this in Jessie

only in constructing them, which our code doesn't do.

0xpatrickdev · 2024-05-13T14:12:16Z

packages/orchestration/src/examples/stakeAtom.contract.js

+        storageNode,
+        icqConnection,
+        // @ts-expect-error only for undelegate, which we do not use
+        timer: harden({}),


Seems worthwhile to include ChainTimerService in start-stakeAtom.js? makeAccount in StakeAtom returns a StakingAccountKit, so it does seem to be used.

packages/orchestration/src/exos/stakingAccountKit.js

0xpatrickdev · 2024-05-13T22:38:33Z

packages/orchestration/src/exos/stakingAccountKit.js

+
+const expect = (actual, expected, message) => {
+  if (actual !== expected) {
+    console.log(message, { actual, expected });


Why not throw an error here?

spec called for no cosmjs types

- stakingAccountKit: - undelegate waits for unbonding time - max clock skew: 10min - factor out helper.amountToCoin - uniform idiom for state, facets - sync types, guards with spec - disable unused-vars on code only for typechecking - avoid decoding discarded responses - staking-ops.test.js: rename from withdraw... - refine test config constant names - WithdrawReward test doesn't depend on Delegate

dckc changed the title ~~Dc dist helper~~ feat: delegate, redelegate on StakingAccountKit May 7, 2024

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/types.d.ts Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/types.d.ts Outdated Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/types.d.ts Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/test/test-withdraw-reward.js Outdated Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/exos/stakingAccountKit.js Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/exos/stakingAccountKit.js Outdated Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/exos/stakingAccountKit.js Outdated Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

packages/orchestration/src/exos/stakingAccountKit.js Outdated Show resolved Hide resolved

dckc commented May 7, 2024

View reviewed changes

dckc force-pushed the dc-dist-helper branch 4 times, most recently from fbe2f1f to db56eca Compare May 10, 2024 21:14

dckc marked this pull request as ready for review May 10, 2024 21:31

dckc requested review from 0xpatrickdev and turadg May 10, 2024 21:31

dckc changed the title ~~feat: delegate, redelegate on StakingAccountKit~~ feat: redelegate, undelegate on StakingAccountKit May 10, 2024

turadg reviewed May 10, 2024

View reviewed changes

0xpatrickdev reviewed May 13, 2024

View reviewed changes

dckc requested review from turadg and 0xpatrickdev May 13, 2024 22:30

0xpatrickdev approved these changes May 13, 2024

View reviewed changes

dckc added 2 commits May 13, 2024 17:57

docs(orchestration): typo

8eb3184

refactor(orchestration): factor out StakingAccountFacet

8d3ed35

dckc added 2 commits May 13, 2024 17:57

chore: narrow return type for redelegate, undelegate

43b8c97

spec called for no cosmjs types

dckc force-pushed the dc-dist-helper branch from dca230b to 66885c0 Compare May 13, 2024 23:00

dckc added automerge:rebase Automatically rebase updates, then merge and removed automerge:rebase Automatically rebase updates, then merge labels May 13, 2024

chore: plumb timer thru stakeAtom.contract

67211a0

dckc added the automerge:rebase Automatically rebase updates, then merge label May 13, 2024

mergify bot merged commit 9553675 into master May 14, 2024
71 checks passed

mergify bot deleted the dc-dist-helper branch May 14, 2024 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: redelegate, undelegate on StakingAccountKit #9331

feat: redelegate, undelegate on StakingAccountKit #9331

dckc commented May 7, 2024 •

edited

cloudflare-pages bot commented May 7, 2024 •

edited

dckc May 7, 2024

turadg May 10, 2024

0xpatrickdev May 13, 2024 •

edited

dckc May 7, 2024 •

edited

turadg May 10, 2024

dckc May 13, 2024

dckc May 7, 2024

dckc May 7, 2024

0xpatrickdev May 7, 2024

dckc commented May 10, 2024

turadg left a comment

turadg May 10, 2024

0xpatrickdev May 13, 2024

dckc May 13, 2024

turadg May 10, 2024

turadg May 10, 2024

dckc May 13, 2024

turadg May 10, 2024

turadg May 10, 2024

0xpatrickdev left a comment

0xpatrickdev May 13, 2024

0xpatrickdev May 13, 2024 •

edited

0xpatrickdev May 13, 2024

dckc May 13, 2024

0xpatrickdev May 13, 2024

0xpatrickdev May 13, 2024

		trace('MsgBeginRedelegateResponse', response);
		// TODO? return completionTime?

		@@ -18,3 +20,9 @@ export const Proto3Shape = {
		};

		export const CoinShape = { value: M.bigint(), denom: M.string() };


		const result = await E(account).executeEncodedTx([
		const result = await E(helper.owned()).executeEncodedTx([

feat: redelegate, undelegate on StakingAccountKit #9331

feat: redelegate, undelegate on StakingAccountKit #9331

Conversation

dckc commented May 7, 2024 • edited

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-pages bot commented May 7, 2024 • edited

Deploying agoric-sdk with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev May 13, 2024 • edited

Choose a reason for hiding this comment

dckc May 7, 2024 • edited

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

dckc commented May 10, 2024

turadg 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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev May 13, 2024 • edited

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

dckc commented May 7, 2024 •

edited

cloudflare-pages bot commented May 7, 2024 •

edited

0xpatrickdev May 13, 2024 •

edited

dckc May 7, 2024 •

edited

0xpatrickdev May 13, 2024 •

edited