Skip to content

Conversation

@svyatonik
Copy link
Contributor

@svyatonik svyatonik commented Feb 18, 2022

closes #1319

The main thing here is EncodedOrDecodedCall, which has the same encoding as on-chain calls. But it has two variants - the call may be decoded (that's what we have been using previously). Or it may be encoded and only needs to be decoded when it needs to. In terms of relay subcommands, it means that if you're using pre-encoded calls in cli options and specify their dispatch weight, we'll never need to decode this call (this is the main problem from #1319 - we can't decode Rococo/Wococo/Kusama/Polkadot calls, because we have no access to the runtime from the relay).

Examples:

svyatonik@xwnotebook:~/dev/rococo-wococo.test$ ./bin/substrate-relay encode-message rococo-to-wococo call --sender 5CUXMS6Ja5E8HGa8qq2jumboFKjqpjA8mrdzyANhk7P2mELp --dispatch-weight 10000000000 raw 00010442
INFO bridge Created Message Payload: MessagePayload {
    spec_version: 9140,
    weight: 10000000000,
    origin: CallOrigin::SourceAccount(
        122a87a73f92704a4556131f8c2d14a32dc1d624fc523ba4e528bff3f9dfe55f (5CUXMS6J...),
    ),
    dispatch_fee_payment: DispatchFeePayment::AtSourceChain,
    call: 0x00010442,
}
INFO bridge Encoded Message Payload: 0xb423000000e40b540200000002122a87a73f92704a4556131f8c2d14a32dc1d624fc523ba4e528bff3f9dfe55f001000010442
0xb423000000e40b540200000002122a87a73f92704a4556131f8c2d14a32dc1d624fc523ba4e528bff3f9dfe55f001000010442
svyatonik@xwnotebook:~/dev/rococo-wococo.test$ ./bin/substrate-relay estimate-fee rococo-to-wococo call --sender 5CUXMS6Ja5E8HGa8qq2jumboFKjqpjA8mrdzyANhk7P2mELp --dispatch-weight 0 raw 00010442
INFO bridge Created Message Payload: MessagePayload {
    spec_version: 9140,
    weight: 0,
    origin: CallOrigin::SourceAccount(
        122a87a73f92704a4556131f8c2d14a32dc1d624fc523ba4e528bff3f9dfe55f (5CUXMS6J...),
    ),
    dispatch_fee_payment: DispatchFeePayment::AtSourceChain,
    call: 0x00010442,
}
INFO bridge Encoded Message Payload: 0xb4230000000000000000000002122a87a73f92704a4556131f8c2d14a32dc1d624fc523ba4e528bff3f9dfe55f001000010442
INFO bridge Fee: Balance(293479234556)
293479234556
svyatonik@xwnotebook:~/dev/rococo-wococo.test$ ./bin/substrate-relay send-message rococo-to-wococo --source-signer //Alice --dispatch-weight 100000000000 raw 00010442
INFO bridge Created Message Payload: MessagePayload {
    spec_version: 9140,
    weight: 100000000000,
    origin: CallOrigin::SourceAccount(
        d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d (5GrwvaEF...),
    ),
    dispatch_fee_payment: DispatchFeePayment::AtSourceChain,
    call: 0x00010442,
}
INFO bridge Encoded Message Payload: 0xb423000000e876481700000002d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d001000010442
INFO bridge Sending message to Wococo. Lane: [0, 0, 0, 0]. Size: 177. Dispatch weight: 100000000000. Fee: 1,173,439,257,663
INFO bridge The source account (d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d (5GrwvaEF...)) balance will be reduced by (at most) 1173439257663 (message fee) + 31605393717 (tx fee	) = 1205044651380 Rococo tokens
INFO bridge Signed Rococo Call: 0xc502bd028400d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d0180c4412df47caab679790c7992755661d891bc73ed75b632a46372ab23e2200fafec0946b26eaa091bbd04671261c06c15911e0f4e1a421d6943c66fc9c3ca850008002c0300000000b423000000e876481700000002d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d0010000104423f1c6e36110100000000000000000000

@svyatonik svyatonik marked this pull request as ready for review February 18, 2022 07:36
@svyatonik
Copy link
Contributor Author

I'll merge this without approval for now - it needs to be tested on Rialto+Millau and we already have a bunch of unreviewed PRs. So maybe it'll be easier to review them in a polkadot-staging-update PR at once.

@svyatonik svyatonik merged commit 1ed0985 into master Feb 21, 2022
@svyatonik svyatonik deleted the estimate-rwkp-calls branch February 21, 2022 08:53
_info: &DispatchInfoOf<Self::Call>,
_len: usize,
) -> Result<Self::Pre, TransactionValidityError> {
Ok(self.validate(who, call, info, len).map(|_| ())?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we forgo validation completely? Is this because we can't validate encoded call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - this function is only called within runtime and we're using SignedExtensions just to get the encoded data (so we don't need any validation at all). More over, the default SignedExtension::validate actually does nothing: https://github.com/paritytech/substrate/blob/master/primitives/runtime/src/traits.rs#L862-L879

fn encode_call(call: &Call) -> anyhow::Result<Self::Call> {
fn encode_call(call: &Call) -> anyhow::Result<EncodedOrDecodedCall<Self::Call>> {
Ok(match call {
Call::Raw { data } => EncodedOrDecodedCall::Encoded(data.0.clone()),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this fn only returns EncodedOrDecodedCall::Encoded variant for Call::Raw, right?

Copy link
Contributor Author

@svyatonik svyatonik Mar 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Why is it important for us - we can't maintain the whole copy of Polkadot/Kusama enum Call here, in substrate relay code. Instead, we allow to encode call externally and use this raw variant. We can't decode it here, because for most of calls, decode will fail (except for this remark). So yes - it only returns EncodedOrDecodedCall::Encoded for Call::Raw

Comment on lines +35 to +37
fn encode_call(call: &Call) -> anyhow::Result<EncodedOrDecodedCall<Self::Call>> {
Ok(match call {
Call::Raw { data } => Decode::decode(&mut &*data.0)?,
Call::Raw { data } => Self::Call::decode(&mut &*data.0)?.into(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, for Millau the same trait fn encode_call() actually decodes the call when raw data is provided?

Why not use EncodedOrDecodedCall::Encoded(data.0.clone()) variant like in Kusama?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be changed. But:

  1. imo earlier error = better error. Since Call of Rialto/Millau is the runtime call (not the mocked call as for the Polkadot/Kusama), decode only fail if the call is invalid. It is better to see decoding error here, than some strange error later (e.g. on the bridged chain during dispatch);
  2. most of times (except when sending messages manually or using sending max-dispatch-weight messages) in Rialto<>Millau bridge, we'll be calling get_dispatch_info() on this call to get its dispatch weight. So it needs to be decoded anyways.

Feel free to insist, if you think it'll be better to change it to the Encoded here, though :)

svyatonik added a commit that referenced this pull request Mar 21, 2022
* Fix mandatory headers scanning in on-demand relay (#1306)

* more logging in on-demand headerss

* remove `maximal_headers_difference` concept from on-demand-relay

* another leftover from previous on-demand version

* removed extra log

* fmt

* increase relay balance guard limits for Polkadot<>Kusama bridge (#1308)

* fix benchmarks before using it in Polkadot/Kusama/Rococo runtimes (#1309)

* Endow relayer account at target chain in message benchmarks (#1310)

* endow relayer account at target chain in message benchmarks

* pick another line

* expose fee multiplier metrics in messages relay (#1312)

* Fix issues from cargo deny (#1311)

* update libp2p-core (RUSTSEC-2022-0009)

* update thread_local (RUSTSEC-2022-0006)

* time 0.2 -> time 0.3

* ignore RUSTSEC-2021-0130

* proper migration to time 0.3

* fix clippy?

* Revert "fix clippy?"

This reverts commit 53bc289.

* Add some tests to check integrity of chain constants + bridge configuration (#1316)

* add some tests to check integrity of chain constants + bridge configuration

* try to use named parameters where possible

* Encode and estimate Rococo/Wococo/Kusama/Polkadot messages (#1322)

* encode and estimate Rococo/Wococo/Kusama/Polkadot messages

* allow send-message for non-bundled chains

* weight -> dispatch-weight

* fmt

* fix spelling

* impl Decode for SignedExtensions (otherwise transaction resubmitter panicks) (#1325)

* use mortal transactions in transaction resubmitter (#1326)

* Using-same-fork metric for finality and complex relay (#1327)

* using_same_fork metric in finality relay

* support `using_different_forks` in messages relay

* added dashboards and alerts

* lockfile

* fix typo in alert expression (#1329)

* removed extra *_RUNTIME_VERSION consts from relay code (#1330)

* Reinitialize bridge relay subcommand (#1331)

* reinitialize bridge subcommand

* PolkadotToKusama in reinit-bridge

* fix clippy issues (#1332)

* Revert "Revert "override conversion rate in estimate-message-fee RPC (#1189)" (#1275)" (#1333)

This reverts commit ed7def6.

* Override conversion rate when computing message fee (#1261)

* override conversion rate when message is sent

* spelling + fmt

* add --conversion-rate-override cli option

* try to read conversion rate from cmd output

* fix output

* fmt

* fi typo in generator script (#1334)

* override conversion rate in tokens swap generator (#1335)

* fix conversion rate override in token swap (#1336)

* synchronize relay cli changes and token swap generator script (#1337)

* fixed mess with conversion rates (#1338)

* fix generator scripts to be consistent with updatedrelay output (#1339)

* Increase rate from metric when estimating fee (#1340)

* ignore errors when dumping logs and container is missing

* fixed typo

* print correct payload length

* increase conversion rate a bit when estimating fee (to avoid message rejects when rate update tx is active)

* fmt

* fix conversion rate metric in dashboards (#1341)

* get rid of '[No Data] Messages from Millau to Rialto are not being delivered' warnings (#1342)

* use DecodeLimit when decoding incoming calls (#1344)

* update regex to 1.5.5 (#1345)

* edition = "2021" (#1346)

* Mortal conversion rate updater transactions (#1257)

* merge all similar update_conversion_rate functions

* stall timeout in conversion rate update loop

* fmt

* fix

* added alerts for relay balances (#1347)

* replace From<>InboundLaneApi with direct storage reads (#1348)

* added no_stack_overflow_when_decoding_nested_call_during_dispatch test (#1349)

* ignore more "increase" alerts that are sometimes signalling NoData at startup (#1351)

* Support dedicated lanes for pallets (#962)

* pass call origin to the message verifier

* is_outbound_lane_enabled -> is_message_accepted

* trait SenderOrigin

* only accept messages from token swap pallet to token swap lane

* tests for edge cases of pay_delivery_and_dispatch_fee

* fixed origin verification

* fmt

* fix benchmarks compilation

* fix TODO with None account and non-zero message fee (already covered by tests)

* revert cargo fmt changes temporarily

* Update Substrate/Polkadot/Cumulus references (#1353)

* cumulus: 4e95228
polkadot: 975e780ae0d988dc033f400ba822d14b326ee5b9
substrate: 89fcb3e4f62d221d4e161a437768e77d6265889e

* fix refs

* sync changes from paritytech/polkadot#3828

* sync changes from paritytech/polkadot#4387

* sync changes from paritytech/polkadot#3940

* sync with changes from paritytech/polkadot#4493

* sync with changes from paritytech/polkadot#4958

* sync with changes from paritytech/polkadot#3889

* sync with changes from paritytech/polkadot#5033

* sync with changes from paritytech/polkadot#5065

* compilation fixes

* fixed prometheus endpoint startup (it now requires to be spawned within tokio context)

* update chain versions (#1358)

* update parity-scale-codec to 3.1.2 (#1359)

* fix parse_transaction on Rialto+Millau (#1360)
svyatonik pushed a commit that referenced this pull request Jul 17, 2023
Bumps [parking_lot](https://github.com/Amanieu/parking_lot) from 0.12.0 to 0.12.1.
- [Release notes](https://github.com/Amanieu/parking_lot/releases)
- [Changelog](https://github.com/Amanieu/parking_lot/blob/master/CHANGELOG.md)
- [Commits](Amanieu/parking_lot@0.12.0...0.12.1)

---
updated-dependencies:
- dependency-name: parking_lot
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
…h#1322)

* encode and estimate Rococo/Wococo/Kusama/Polkadot messages

* allow send-message for non-bundled chains

* weight -> dispatch-weight

* fmt

* fix spelling
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
…h#1322)

* encode and estimate Rococo/Wococo/Kusama/Polkadot messages

* allow send-message for non-bundled chains

* weight -> dispatch-weight

* fmt

* fix spelling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An easy way to estimate Kusama <> Polkadot message fee

3 participants