Delivery fee should cover cost of local processing #975

yrong · 2023-10-20T07:07:17Z

We need to take base fee(fee in bridgehub) into consideration, more context in #968 (comment)

Co-authored-by: Vincent Geddes <[email protected]>

…into ron/sno-632

parachain/pallets/outbound-queue/src/lib.rs

vgeddes · 2023-10-25T17:32:55Z

Lets limit the scope of this PR, as it does solve a few important problems:

Benchmarks for processing a message and committing a single message.
Charging base_fee + delivery_fee (See my comment in Delivery fee should cover cost of local processing #975 (comment) why I want this to be the default)
Remove all voucher/redeem and estimate_fee runtime API.

alistair-singh

Looks good!

parachain/pallets/control/src/tests.rs

yrong · 2023-10-26T02:49:24Z

Remove all voucher/redeem and estimate_fee runtime API.

Hey, Vincent. Actually voucher/redeem has been removed in previous commit, but for estimate_fee I prefer to keep as it could be helpful for chains to estimate fee in advance before a real transfer. Also the return of the runtime api includes both base_fee and delivery_fee which is not obvious in current impl.

vgeddes · 2023-10-26T12:01:44Z

Remove all voucher/redeem and estimate_fee runtime API.

Hey, Vincent. Actually voucher/redeem has been removed in previous commit, but for estimate_fee I prefer to keep as it could be helpful for chains to estimate fee in advance before a real transfer. Also the return of the runtime api includes both base_fee and delivery_fee which is not obvious in current impl.

OK, lets keep estimate_fee.

parachain/pallets/control/src/lib.rs

parachain/pallets/control/src/mock.rs

parachain/pallets/outbound-queue/runtime-api/src/lib.rs

parachain/pallets/outbound-queue/src/api.rs

parachain/primitives/core/src/outbound.rs

parachain/primitives/router/src/outbound/mod.rs

parachain/primitives/core/src/outbound.rs

parachain/pallets/outbound-queue/src/lib.rs

parachain/pallets/outbound-queue/runtime-api/src/lib.rs

parachain/pallets/outbound-queue/src/api.rs

parachain/pallets/outbound-queue/runtime-api/src/lib.rs

Co-authored-by: Vincent Geddes <[email protected]>

vgeddes

Last round of minor comments

vgeddes · 2023-10-27T12:38:22Z

parachain/pallets/outbound-queue/src/api.rs

+pub fn calculate_fee<Runtime>(
+	message: Message,
+) -> Result<Fees<<Runtime as Config>::Balance>, SubmitError>


Hmm, actually, should rename Runtime to T. They are the same thing really. And I think its not necessary to cast Runtime into Config.

Suggested change

pub fn calculate_fee<Runtime>(

message: Message,

) -> Result<Fees<<Runtime as Config>::Balance>, SubmitError>

pub fn calculate_fee<T>(

message: Message,

) -> Result<Fees<T::Balance>, SubmitError>

Rename generic Runtime to T

parachain/pallets/outbound-queue/src/api.rs

vgeddes · 2023-10-27T12:40:36Z

parachain/pallets/outbound-queue/src/lib.rs

+				id: message_id,
+				origin: message.origin,
+				message: encoded,
+				fee,


I'm not seeing the OutboundQueueTicket.fee field being used anywhere. Can it removed?

Remove fee from OutboundQueueTicket

vgeddes · 2023-10-27T12:42:30Z

parachain/primitives/router/src/outbound/mod.rs

@@ -132,7 +132,7 @@ where
 		})?;

 		// convert fee to MultiAsset
-		let fee = MultiAsset::from((MultiLocation::parent(), fee)).into();
+		let fee = MultiAsset::from((MultiLocation::parent(), fee.total())).into();


Suggested change

let fee = MultiAsset::from((MultiLocation::parent(), fee.total())).into();

let fee = MultiAsset::from((MultiLocation::parent(), fees.total())).into();

Rename to fees

yrong and others added 30 commits August 31, 2023 13:45

Submit outbound message without MessageQueue

1c37553

Priority control

ac791fd

Update priority

be1bf80

Update cumulus

d9d1ee8

Merge branch 'main' into ron/sno-632

e142adf

Reserve space for high priority messages

4cf60f8

Fix compile error & More refactor

d4ea301

Merge branch 'main' into ron/sno-632

e9c4b5b

Merge branch 'main' into ron/sno-632

b370fb1

Improve processing high priority messages

d4748ca

Some polish

d72b493

Add priority back

16b369b

Improve comments

477ece8

Merge branch 'main' into ron/sno-632

4993752

Rename priority to normal

cc47ac5

Refactor submit logic

db7cf6b

Refactor process message

15957d1

Add type HighPriorityCommands

7252285

Fix clippy

cfb1cfb

Rename priority with BridgeHub

783762c

Update parachain/primitives/core/src/outbound.rs

3bf3f88

Co-authored-by: Vincent Geddes <[email protected]>

Update parachain/pallets/outbound-queue/src/lib.rs

370aa03

Co-authored-by: Vincent Geddes <[email protected]>

Update parachain/pallets/outbound-queue/src/lib.rs

9bdd2b9

Co-authored-by: Vincent Geddes <[email protected]>

Merge branch 'ron/sno-632' of https://github.com/Snowfork/snowbridge …

2c3d2d0

…into ron/sno-632

Merge branch 'main' into ron/sno-632

167b250

Merge branch 'main' into ron/sno-632

876425c

Revamp with ExportOrigin

a6317f0

Merge branch 'main' into ron/sno-632

a593a66

Fix format

de5edc5

Some polish

79e5d4e

Benchmark for do_submit_message

dfb58a3

yrong commented Oct 24, 2023

View reviewed changes

parachain/pallets/outbound-queue/src/lib.rs Outdated Show resolved Hide resolved

alistair-singh reviewed Oct 25, 2023

View reviewed changes

parachain/pallets/control/src/tests.rs Outdated Show resolved Hide resolved

yrong added 3 commits October 26, 2023 09:44

Add delivery_fee

8280634

Improve fee test

9037a8c

Merge branch 'main' into ron/base-fee

c43cbd9

vgeddes reviewed Oct 26, 2023

View reviewed changes

yrong added 7 commits October 27, 2023 00:46

Remove benchmark for submit

3f685ea

Add comments

95d673a

Rename to calculate_fee

8d6ec9d

Comments

ae1cbd5

Add method for total fee

727288d

Short name

f864192

Rename to commit_messages

6691413

vgeddes reviewed Oct 27, 2023

View reviewed changes

parachain/pallets/outbound-queue/runtime-api/src/lib.rs Outdated Show resolved Hide resolved

parachain/pallets/outbound-queue/runtime-api/src/lib.rs Outdated Show resolved Hide resolved

parachain/pallets/outbound-queue/src/api.rs Outdated Show resolved Hide resolved

Use Balance generic for the api

61d1df5

vgeddes reviewed Oct 27, 2023

View reviewed changes

parachain/pallets/outbound-queue/runtime-api/src/lib.rs Outdated Show resolved Hide resolved

Update parachain/pallets/outbound-queue/runtime-api/src/lib.rs

8f652cf

Co-authored-by: Vincent Geddes <[email protected]>

vgeddes reviewed Oct 27, 2023

View reviewed changes

yrong added 3 commits October 27, 2023 21:13

Rename generic Runtime to T

5548e94

Remove fee from OutboundQueueTicket

8fa7dac

Rename to fees

356d17c

vgeddes changed the title ~~Base fee~~ Delivery fee should cover costs of local processing Oct 27, 2023

vgeddes changed the title ~~Delivery fee should cover costs of local processing~~ Delivery fee should cover cost of local processing Oct 27, 2023

vgeddes approved these changes Oct 27, 2023

View reviewed changes

yrong merged commit 187c3ff into main Oct 27, 2023
5 checks passed

yrong deleted the ron/base-fee branch October 27, 2023 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delivery fee should cover cost of local processing #975

Delivery fee should cover cost of local processing #975

yrong commented Oct 20, 2023 •

edited

Loading

vgeddes commented Oct 25, 2023

alistair-singh left a comment

yrong commented Oct 26, 2023 •

edited

Loading

vgeddes commented Oct 26, 2023

vgeddes left a comment

vgeddes Oct 27, 2023

yrong Oct 27, 2023

vgeddes Oct 27, 2023

yrong Oct 27, 2023

vgeddes Oct 27, 2023

yrong Oct 27, 2023

	let fee = MultiAsset::from((MultiLocation::parent(), fee.total())).into();
	let fee = MultiAsset::from((MultiLocation::parent(), fees.total())).into();

Delivery fee should cover cost of local processing #975

Delivery fee should cover cost of local processing #975

Conversation

yrong commented Oct 20, 2023 • edited Loading

vgeddes commented Oct 25, 2023

alistair-singh left a comment

Choose a reason for hiding this comment

yrong commented Oct 26, 2023 • edited Loading

vgeddes commented Oct 26, 2023

vgeddes left a comment

Choose a reason for hiding this comment

vgeddes Oct 27, 2023

Choose a reason for hiding this comment

yrong Oct 27, 2023

Choose a reason for hiding this comment

vgeddes Oct 27, 2023

Choose a reason for hiding this comment

yrong Oct 27, 2023

Choose a reason for hiding this comment

vgeddes Oct 27, 2023

Choose a reason for hiding this comment

yrong Oct 27, 2023

Choose a reason for hiding this comment

yrong commented Oct 20, 2023 •

edited

Loading

yrong commented Oct 26, 2023 •

edited

Loading