Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

Rewrite post$ to take a password and not use signer #93

Merged
merged 12 commits into from
Jan 22, 2019
Merged

Conversation

axelchalon
Copy link
Contributor

Breaking change to the post$ API (before: tx, options; now: tx, passphrase, options). Should bump version consequently.

post$ now doesn't require any subsequent action from the caller (no more signer_ rpcs). post$ uses personal_signTransaction with the tx and passphrase to get the signed tx, and calls postRaw$ with it.

Con: AFAIK the personal api needs to be activated in Parity's launch flag --ws-apis (unless the secure token covers this; haven't checked).

Copy link
Collaborator

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

Could you also bump all @parity/* packages and deps to ^4.0.0?

observer.next({ confirmed: receipt });
}
const signedTransaction = await api.personal.signTransaction(tx, passphrase);
postRaw$(signedTransaction.raw).subscribe(observer);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is a breaking change, what do you think of putting observer.next({ signed: ... }) here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good!

const { provider } = options;
const api = provider ? createApiFromProvider(provider) : getApi();

const source$ = Observable.create(async (observer: Observer<TxStatus>) => {
try {
const transactionHash = await api.eth.sendRawTransaction(tx);
const transactionHash = await api.eth.sendRawTransaction(rawTx);
observer.next({ signed: transactionHash });
Copy link
Collaborator

Choose a reason for hiding this comment

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

... and changing here to observer.next({ sent: ... })

@amaury1093 amaury1093 mentioned this pull request Jan 16, 2019
@amaury1093 amaury1093 added the breaking Breaking change label Jan 16, 2019
Copy link
Collaborator

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

Conflicts need to be solved. Can you also bump inside lerna.json?

@axelchalon axelchalon added the WIP label Jan 18, 2019
@axelchalon axelchalon removed the WIP label Jan 21, 2019
Copy link
Collaborator

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

lgtm!

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

Successfully merging this pull request may close these issues.

None yet

2 participants