semuxproject / semux-core

Semux Core
https://www.semux.org
MIT License
76 stars 31 forks source link

API: /broadcast-raw-transaction should be POST, not GET #275

Closed witoldsz closed 3 years ago

witoldsz commented 5 years ago

Broadcasting a transaction is a stateful, non-idenpotent operation on the blockchain and GET operations are not supposed to be like this. Different surprising results might be encountered by browsers and/or proxy software (for example: trying to deploy a contract twice migth result in deploying it once and having the same result send back to caller twice).

Aside from potential issues, it's also misleading developers.

My suggestion would be to deprecate (or just get rid of) GET /broadcast-raw-transaction and introduce a POST one.

semuxgo commented 3 years ago

From the blockchain's perspective, /broadcast-raw-transaction is stateful as it might introduce changes to the global state. However, it does not necessarily have an impact on the node that vendors the API services. What it does is essentially to relay the transaction to the network.

Given this is compatbility-breaking API change, I would hesitate to implement this for now unless it's causing too much pain. With that said, I'd keep this ticket open for future enhancement consideration.

semuxgo commented 3 years ago

Quick turnaround

As I'm removing deprecated APIs, also compatibility-breaking, I've included the proposed change to https://github.com/semuxgo/semux-core/commit/a1509bdaf28ac0871e2c8f2eb45c968ded4e3622