Staking changes

[apporc]

No description provided.

[cloudflare-workers-and-pages]

Deploying 2nicove with    Cloudflare Pages

Latest commit:	0194b5f
Status:	🚫  Build failed.

View logs

[dafuga]

Suggested change

	return Asset.fromUnits(claimable, network.chain.systemToken!.symbol);
	return Asset.fromUnits(claimable, network.chain.systemToken?.symbol || '4,UNKNOWN);

[dafuga]

I would do something like this everywhere that you're using the ! operand.

[apporc]

Just checked that we are using this.chain.systemToken.symbol almost everywhere, without ! or ?. Your IDE doesn't report the error or you ignore it?

[dafuga]

Oh yeah, that was my bad, it was because I was using an old version of the wharfkit/common package 😅 I have that all cleaned up in my latest PR 👍

[dafuga]

I still think that this doesn't belong in a state file.

[apporc]

I could use another class name like WithdrawManager, instead of xxxState, lol. Yeah i agree a pure state should not hold more logics, but here i think what we need is not a pure state. I know how you've wrote other pages, like ram and send pages, read all about them before coding this, but i think this approach in staking is more clear and easy. Tell me which one you like, WithdrawManager or a pure state and dissecting it in another page file? Keep in mind that, there is consistently only 5 lines of code in every page file, it's fully abstraction between ui and logic :

	const { data } = $props();

	let stakeState: StakeState = $state(new StakeState(data.network));

	$effect(() => {
		stakeState.sync(data.network, context.account, context.wharf);
	});

[dafuga]

Renaming it would be a step in the right direction, but my other issue with your approach is that it's not consistent with what we've been doing on the other pages.

[dafuga]

How about you follow the state class format that we have in the other pages, but you can also have another class to manage the action handling? Maybe it can even take a state class instance as a param? The result would be similar to what you have here, but more consistent with the other pages.

[apporc]

I don't think state and state manager should be separated. They are just class's members and class's method properties. Why would we make them two classes?
If you think the withdrawManager is the right direction, why not using this approach in all other pages?