Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix transaction submission for hex #259

Merged
merged 2 commits into from
Jun 28, 2018
Merged

Fix transaction submission for hex #259

merged 2 commits into from
Jun 28, 2018

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Jun 27, 2018 •
edited by pepyakin
Loading

Closes #249

@gavofyork gavofyork requested a review from jacogr June 27, 2018 16:00
@gavofyork gavofyork added the A0-please_review Pull request needs code review. label Jun 27, 2018
pepyakin
pepyakin reviewed Jun 27, 2018
View reviewed changes
substrate/rpc/src/author/error.rs Outdated
@@ -24,6 +24,11 @@ error_chain! {
Pool(txpool::Error, txpool::ErrorKind) #[doc = "Pool error"];
}
errors {
/// Not implemented yet
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the comment is wrong.

substrate/primitives/src/lib.rs Outdated
@@ -87,3 +89,17 @@ pub type AuthorityId = [u8; 32];

/// A 512-bit value interpreted as a signature.
pub type Signature = hash::H512;

/// Contract storage key.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this says "contract storage key"?

substrate/rpc/src/author/mod.rs
@@ -35,8 +37,11 @@ build_rpc_trait! {
/// Substrate authoring RPC API
pub trait AuthorApi<Hash, Extrinsic> {
/// Submit extrinsic for inclusion in block.
#[rpc(name = "author_submitRichExtrinsic")]
fn submit_rich_extrinsic(&self, Extrinsic) -> Result<Hash>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about naming scheme choosen here. I kinda understand why it is "rich" extrinsic, but maybe it is better to leave it as "submit_extrinsic" and make another procedure call "submit_raw_extrinsic"?

Or are we constrained by backward compatibility here?

Copy link
Contributor

Choose a reason for hiding this comment

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

most practical uses of substrate will be easier to use with the raw variant, so it makes sense to me to optimize the API a bit for it.

jacogr
jacogr approved these changes Jun 28, 2018
View reviewed changes
Copy link
Contributor

@jacogr jacogr left a comment

Choose a reason for hiding this comment

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

Tested against UI and it now correctly accepts the hex-encoding again.

(Only issues in this flow I found looks to be related to UI enumerated account support which itself seems not fully compliant to the implementation)

@pepyakin pepyakin added A8-looksgood and removed A0-please_review Pull request needs code review. labels Jun 28, 2018
@jacogr
Copy link
Contributor

jacogr commented Jun 28, 2018

Just one comment -just checking if it is as-intended. There is a mismatch here with regards to the previous iteration. Previously could submit just the data hex, i.e 0xff7769..., now it would seem it needs 0x<length>ff7769...

@jacogr jacogr mentioned this pull request Jun 28, 2018
Merged
@gavofyork
Copy link
Member Author

that's intended - it's the full form of the transaction.

@gavofyork gavofyork merged commit 0ad8b40 into master Jun 28, 2018
@gavofyork gavofyork deleted the gav-submit-trans branch June 28, 2018 14:42
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
@gui1117 @rphmeier
update substrate (paritytech#259)
ee0076f 
* WIP

* merging select_chain

* WIP

* update to point to gui-polkadot-master

* Fix collator

* update gui-polkadot-master and fix

* fix unwraps

* better returning an error
JoshOrndorff added a commit to moonbeam-foundation/substrate that referenced this pull request Apr 21, 2021
@notlesh @JoshOrndorff @girazoki
v0.7 dependency updates (paritytech#259)
e6f98b5 
* Use official ethereum crate at version 0.7

* Point frontier at v0.7-moonbeam

* Bump Ethereum, cumulus/polkadot/substrate

Ethereum: v0.7
cumulus: 28d1d00747e1e60a3e26f0112b4580c805fa1557
polkadot: ce03f37
substrate: ca63242

* s/cumulus-primitives/cumulus-primitives-core/

* Remove cumulus-runtime; bump substrate-wasm-builder version

* Rename cumulus packages and update Cargo.lock

* Reflect rename of cumulus_primitives -> cumulus_primitives

* s/cumulus_service/cumulus_client_service/

* Update rust-toolchain from 2020-10-03 -> 2021-01-13

* More renaming to reflect upstream refactors

* Bump frontier c364ac4 -> 0db6896

* Bump frontier from 0bd068ab -> cf4be1a

* Update Cargo.lock

* Reflect upstream naming changes

* Update Cargo.lock

* Reflect more upstream name changes

* More fixes

* Reflect StartCollatorParams refactor

* Reflect ParachainInherentData refactor

* Bump frontier from cf4be1a -> 23f8442

* Use ethereum from current master (commit f11532e)

* Remove dead code

* cargo fmt

* fix author inherent mock

* fix stake mock and tests

* cargo fmt

* oops, how did dev_service.rs come back?

* Pass the wrapped FrontierBlockImport to build_relay_chain_consensus

* Update dependencies so as not to conflict with substrate

* Bump deps to avoid conflict with polkadot/cumulus

* Reflect frame_support logging refactor

* Point ethereum dependency back to official crate

* Point frontier branch to notlesh-substrate-a8c2bc66

* Point substrate back to a8c2bc6

* Bump substrate, polkadot, and cumulus

* Point frontier to correct branch in Cargo.toml files

* Update Cargo.lock

* Update RandomnessSource to include BlockNumber type

* Reflect Cumulus change to register_validate_block

* Add OnChargeTransaction

* Use polkadot commit c8649e5

* Point frontier to new PR

* Use frontier branch 1cf3f5b

* Update frontier to 3c0c13e

* Pass std feature through grandpa, update Cargo.lock

* Point frontier at 7866ef0

* Point frontier to d214085

* Point frontier to 6f48d04

* Disable logging in runtime on release build

* Point frontier to a65d617

* Incremental service refactor

* More incremental service.rs refactor

* Update trait impl method signature and unused telemetry argument

* Remove wrapping Option in GenesisConfig

* Add frontier_backend

* Add overrides to EthApi

* Remove dead code

* Point polkadot to PureStake/polkadot @ moonbeam-v0.7

* Point cumulus at PureStake/cumulus @ moonbeam-v0.7

* Bump binary version to v0.7.0

* Point frontier to PureStake/notlesh-moonbeam-v0.7

* Point frontier to 3fde366

* Updates script tu support purestake/polkadot repo

* Bump Frontier commit

* Point all substrate crates at 97ecd62

* Update Cargo.lock

* Point cumulus, polkadot, frontier, and substrate at purestake repos

* Hopeful fix for fc-db usage

* Point Cargo.toml files to purestake repos

* Update frontier from 383eb23 -> 52c691f

* Fixes for fc-mapping-sync

* Initialize frontier-mapping-sync-worker in dev service

* Update @polkadotjs/apps

* Update tests to reflect totalDiffullty returning 0 instead of null

* Substrate to rococo-v1#1404f2af

* prettier

* Fix test mocks

* master.into() // but staking tests fail. I probably botched that part of the merge.

* Fix staking tests

I'm not sure why the index changed, but it doesn't seem relevant to what 
we're trying to test. Ultimately we got the same error that Amar was 
expecting.

* little cleanup

* Fixes launching scripts

* Revert "Fixes launching scripts"

This reverts commit 53dca638b4d190fb4ad77a34225fc90d3adec4ee.

* Updates spec template for parachainStaking

* Both services compile

* Fix formatting

* Update moonbeam-types-bundle/package-lock.json

* Update gas limit used in subscription tests

* Update another low gas value in test-filter-api.ts

* Point to net_peerCount hex string fix in frontier

* Adds test failing contract should not generate log

* Moves test to the end to avoid breaking ordered tests

* point at runtime net version getter

* Generic BlockNumber type

* cargo fmt

* log version 0.4 and minor cleanup

* Crude attempt at documentating the updated gas cost of test transaction

* Revert change to alphanet parachain execution mode

* More thorough approach to magic gas constant

* [so much] prettier

* Prettier still

* Bump spec_version from 26 -> 27

Co-authored-by: Joshy Orndorff <admin@joshyorndorff.com>
Co-authored-by: Crystalin <alan@purestake.com>
Co-authored-by: Gorka Irazoqui <gorka.irazoki@gmail.com>
Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
liuchengxu pushed a commit to chainx-org/substrate that referenced this pull request Aug 23, 2021
Feature/0.6assets rpc (paritytech#259)
c814bf3 
* provide withdrawallog and state

* 0.6 all assets rpc

* provide withdrawal depost list rpc interface

* change listing to online in assets rpc

* fix compile err

* do not remove revoke token in asset_list
liuchengxu pushed a commit to autonomys/substrate that referenced this pull request Jun 3, 2022
@nazar-pc
Merge pull request paritytech#259 from subspace/upgrade-substrate-sna…
5bd9194 
…pshot-2022-02-16

Upgrade substrate to snapshot 2022-02-16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rphmeier rphmeier rphmeier left review comments

@jacogr jacogr jacogr approved these changes

@pepyakin pepyakin pepyakin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gavofyork @jacogr @pepyakin @rphmeier