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

Update signing issue for eth_sendTransactionAsync RPC call #541

Merged
merged 14 commits into from
Oct 8, 2018
Merged

Update signing issue for eth_sendTransactionAsync RPC call #541

merged 14 commits into from
Oct 8, 2018

Conversation

prd-fox
Copy link
Contributor

@prd-fox prd-fox commented Sep 27, 2018

The eth_sendTrasactionAsync RPC call was implementing it's own logic to sign and submit a transaction to the pool, and the signing process wasn't updated for EIP-155. This meant private transactions were being incorrectly signed and got stuck in the TX pool.

It makes sense for the async version of eth_sendTransaction to do the same, but inside a goroutine so it can be executed whilst returning from the RPC call.

This PR also adds the missing required fields from the incoming message (the restriction field) and the to the outgoing callback message (the id field).

prd-fox and others added 6 commits August 16, 2018 15:20
Update request/response data structs to include extra fields

Refactor the SendTransactionAsync method to just call the
SendTransaction method in a goroutine, instead of reimplementing logic
for a new transaction.
when the channel is full. Now it returns immediately with an error
indicating the channel is overloaded.
fixanoid
fixanoid previously approved these changes Oct 1, 2018
@Krish1979 Krish1979 closed this Oct 1, 2018
@Krish1979 Krish1979 reopened this Oct 1, 2018
fixanoid
fixanoid previously approved these changes Oct 5, 2018
@jpmsam jpmsam merged commit 4d015bb into Consensys:master Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants