Skip to content
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

Crab Network claim rewards has a high txfee #473

Closed
WoeOm opened this issue Jul 14, 2020 · 12 comments · Fixed by #472
Closed

Crab Network claim rewards has a high txfee #473

WoeOm opened this issue Jul 14, 2020 · 12 comments · Fixed by #472
Assignees

Comments

@WoeOm
Copy link

WoeOm commented Jul 14, 2020

image

staking.payoutStakers

@AurevoirXavier AurevoirXavier self-assigned this Jul 14, 2020
@AurevoirXavier AurevoirXavier transferred this issue from darwinia-network/darwinia-common Jul 14, 2020
@AurevoirXavier AurevoirXavier linked a pull request Jul 14, 2020 that will close this issue
4 tasks
@AurevoirXavier
Copy link
Member

AurevoirXavier commented Jul 15, 2020

The WeightToFee formula is correct.

But seems the bytes fee not working in previous version (don't know why).

@hackfisher
Copy link
Contributor

The WeightToFee formula is correct.

But seems the bytes fee not working in previous version (don't know why).

Might related to this issue: #447

Will progress after substrate dep get upgraded.

@AurevoirXavier
Copy link
Member

AurevoirXavier commented Jul 15, 2020

No. The root cause is bytes fee. Not relate to weight.

Because the multiplier is still -1 in Crab.

@hackfisher
Copy link
Contributor

hackfisher commented Jul 15, 2020

I mean bytes fees are included in the adjustable fees, and multiplier is not working, could this be the reason why byte fees not working?

@AurevoirXavier
Copy link
Member

But the fix for #447 was not included in RC2.

After I change the bytes fee from 10 * MILLI to 10 * MICRO, it looks good to me.

@AurevoirXavier
Copy link
Member

xavier:
I check the formula. It's correct.
But I found the bytes fee is very expensive. And I didn't change that field.
Seems all the previous version didn't count bytes fee.

shawntabrizi:
👍️
i think byte fee was always counted, but with that PR you linked
we made it so that byte fee
is not effected by the multiplier
so byte fee before used to go to zero
and now it does not
but idk if you have the version where this matters

Don't know why.

So the solution is upgrade to RC4. And a hot fix is need, to low down the tx fee for Crab before the RC4 apply.

@hackfisher
Copy link
Contributor

@AurevoirXavier
Copy link
Member

#[test]
fn Multiplier() {
	let len_fee: u128 = 50;
	let weight_fee: u128 = 50;
	let adjustable_fee = len_fee.saturating_add(weight_fee);
	println!("adjustable_fee: {:?}", adjustable_fee);

	let targeted_fee_adjustment: Multiplier = Multiplier::saturating_from_integer(-1);
	println!("targeted_fee_adjustment: {:?}", targeted_fee_adjustment);

	let adjusted_fee: u128 =
		targeted_fee_adjustment.saturating_mul_acc_int(adjustable_fee.saturated_into());
	println!("adjusted_fee: u128: {}", adjusted_fee);

	let adjusted_fee: i128 =
		targeted_fee_adjustment.saturating_mul_acc_int(adjustable_fee.saturated_into());
	println!("adjusted_fee: i128: {}", adjusted_fee);

	// FIXME: type annotations needed
	// 	adjusted_fee -> ?
	// 	adjusted_fee.saturated_into() -> u128
	//
	// ```rust
	// let adjusted_fee =
	// 	targeted_fee_adjustment.saturating_mul_acc_int(adjustable_fee.saturated_into());
	// let _infer: u128 = 1;
	// let _infer: u128 = _infer.saturating_add(adjusted_fee.saturated_into());
	// ```
}

@hackfisher
Copy link
Contributor

AurevoirXavier added a commit that referenced this issue Jul 15, 2020
AurevoirXavier added a commit that referenced this issue Jul 15, 2020
@hackfisher
Copy link
Contributor

There is a temp workaround by changing byte fee to 0, need to change back after this issue fixed or substrate get upgraded:

https://github.com/darwinia-network/darwinia/pull/472/files#diff-c853329fb7e0f885349d61e4b8c76080R164

1 similar comment
@hackfisher
Copy link
Contributor

There is a temp workaround by changing byte fee to 0, need to change back after this issue fixed or substrate get upgraded:

https://github.com/darwinia-network/darwinia/pull/472/files#diff-c853329fb7e0f885349d61e4b8c76080R164

hackfisher pushed a commit that referenced this issue Jul 15, 2020
* add: scheduler

* update: deps and apply staking hot fix

* rename: `.darwiniarc`

* update: bump chain spec

* remove: bootnodes

* hotfix: #473
@AurevoirXavier
Copy link
Member

Fixed: paritytech/substrate#6145

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants