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

Revisions rpc #30

Merged
merged 59 commits into from
Feb 15, 2021
Merged

Revisions rpc #30

merged 59 commits into from
Feb 15, 2021

Conversation

IntegralTeam
Copy link

Revisions:

  • .gitignore:
    • Added api.rest. - Makefile:
    • Added run-janus, run-qtum, follow-qtum-logs, open-qtum-bash, stop-qtum, restart-qtum rules. - go.mod / go.sum:
    • Called go mod tidy. - pkg/eth/rpc_types.go:
    • Added V, R, S fields at GetTransactionByHashResponse{};
    • Refactored GetTransactionByHashRequest{}.UnmarshalJSON() - added additional checks and error descriptions;
    • Added TODO at GetTransactionReceiptResponse{}.Root field;
    • Changed GetBlockByNumberResponse{} type to be equal to GetBlockByHashResponse{};
    • Refactored GetBlockByNumberRequest{}.UnmarshalJSON() - added additional checks and error descriptions;
    • Added GetBlockByHashRequest{};
    • Added GetBlockByHashRequest{}.UnmarshalJSON();
    • Added GetBlockByHashResponse{};
    • Added newErrInvalidParameterType(); - pkg/qtum/btcasm.go:
    • Applied not necessary code style changes at ParseCreateSenderASM(). - pkg/qtum/client.go:
    • Added additional description to returning errors at Client{}.Request();
    • Changed returned error value to be Qtum's default or Janus's internal (implemented) at Client{}.Do(); - pkg/qtum/jsonrpc.go:
    • Added MethodGetRawTransaction, MethodGetTransactionOut constants;
    • Added JSONRPCError{}.TryGetKnownError();
    • Implemented ErrInvalidAddress var;
    • Added TODO to implement additional error vars. - pkg/qtum/method.go:
    • Added Method{}.GetTransaction();
    • Added Method{}.GetRawTransaction();
    • Applied not necessary code style changes at Method{}.GetTransactionReceipt();
    • Added Method{}.GetTransactionOut();
    • Added Method{}.GetBlockChainInfo(). - pkg/qtum/rpc_types.go:
    • Added genesisBlockHeight, DefaultBlockGasLimit, ZeroAddress, ZeroUserInput constants;
    • Added DecodedRawTransactionResponse{}.CalcAmount();
    • Added ContractInfo{};
    • Added DecodedRawTransactionResponse{}.ExtractContractInfo();
    • Added DecodedRawTransactionResponse{}.IsContractCreation();
    • Added GetTransactionOutRequest{};
    • Added GetTransactionOutResponse{};
    • Refactored GetTransactionReceiptResponse{}.UnmarshalJSON();
    • Added GetRawTransactionRequest{};
    • Added GetRawTransactionResponse{};
    • Partially added RawTransactionVin{};
    • Partially added RawTransactionVout{};
    • Added GetRawTransactionRequest{}.MarshalJSON();
    • Added GetRawTransactionResponse.IsPending();
    • Added GetTransactionResponse{}.IsPending();
    • Added GetBlockHeaderResponse{}.IsGenesisBlock(). - pkg/server/handler.go:
    • Changed error value to be logged from it's cause to a full internal value at httpHandler(). - pkg/transformer/eth_call.go:
    • Changed returning qtum.CallContractRequest{}.GasLimit value to big.NewInt(10000) at ProxyETHCall{}.ToRequest() - TODO added. - pkg/transformer/eth_getBlockByHash.go:
    • Added eth_getBlockByHash.go file;
    • Added ProxyETHGetBlockByHash{};
    • Added ProxyETHGetBlockByHash{}.Method();
    • Added ProxyETHGetBlockByHash{}.Request();
    • Added ProxyETHGetBlockByHash{}.request();
    • Added TODOs and temporary stubs for missing values. - pkg/transformer/eth_getBlockByNumber.go:
    • Changed ProxyETHGetBlockByNumber{}.request() to use ProxyETHGetBlockByHash{}.request() result. - pkg/transformer/eth_getBlockByNumber_test.go:
    • Updated to fit recent struct changes at GetBlockByNumberRequest{}, no logic changes. - pkg/transformer/eth_getFilterChanges.go:
    • Applied renaming changes ProxyETHGetFilterChanges{}.doSearchLogs();
    • Applied renaming changes ProxyETHGetFilterChanges{}.toSearchLogsReq(). - pkg/transformer/eth_getLogs.go:
    • Commented check for len(req.Topics) != 0 at ProxyETHGetLogs{}.Request() - TODO added;
    • Renamed getQtumBlockNumber() to getBlockNumberByParam(), changed and moved to pkg/transformer/util.go;
    • Renamed isString() to isBytesOfString(), changed and moved to pkg/transformer/util.go;
    • Renamed getEthLogs() to extractETHLogsFromTransactionReceipt() and moved to pkg/transformer/util.go. - pkg/transformer/eth_getTransactionByHash.go:
    • Added check for eth.GetTransactionByHashRequest (presents hash string) to be not empty;
    • Changed ProxyETHGetTransactionByHash{}.request() not to drop empty response error and to use getTransactionByHash() instead of direct request;
    • Added getTransactionByHash() - TODOs added;
    • Added getRewardTransactionByHash() - TODOs added;
    • Removed ProxyETHGetTransactionByHash{}.ToRequest(). - pkg/transformer/eth_getTransactionByHash_test.go:
    • Applied renaming changes TestGetTransactionByHashRequest();
    • Applied renaming changes TestGetTransactionByHashRequest_PrecisionOverflow(). - pkg/transformer/eth_getTransactionReceipt.go:
    • Added check for eth.GetTransactionReceiptRequest (presents hash string) to be not empty;
    • Updated ProxyETHGetTransactionReceipt{}.request() - added check and eth.GetTransactionReceiptResponse{}.To/ContractAddress values modifications if transaction is a contract creation;
    • Added TODOs at ProxyETHGetTransactionReceipt{}.request();
    • Remove ProxyETHGetTransactionReceipt{}.ToRequest(). - pkg/transformer/eth_net_version.go:
    • Changed default switch case to set networkID = "0x1024"
      at ProxyETHNetVersion.request() - TODO added. - pkg/transformer/eth_newFilter.go:
    • Replaced old getQtumBlockNumber() with getBlockNumberByParam() at ProxyETHNewFilter{}.request(). - pkg/transformer/transformer.go:
    • Added additional descriptions to returning error values at Transformer{}.Transform(). - pkg/transformer/util.go:
    • Renamed QtumAmountToEthValue() to formatQtumAmount();
    • Added getNonContractTxSenderEthAddress();
    • Added findNonContractTxReceiverEthAddress() - TODO added;
    • Added getBlockNumberByHash();
    • Added getTransactionIndexInBlock();
    • Added formatQtumNonce();
    • Added getBlockNumberByParam() (which is old getQtumBlockNumber() from pkg/transformer/eth_getLogs.go) - added separate earliest and pending switch cases;
    • Added isBytesOfString() (which is old isString() from pkg/transformer/eth_getLogs.go) - added additional checks;
    • Added extractETHLogsFromTransactionReceipt() (which is old getEthLogs() from pkg/transformer/eth_getLogs.go). - pkg/transformer/util_test.go:
    • Changed name from QtumAmountToEthValue() to formatQtumAmount() at TestQtumAmountToEthValue() - no logic changes.

@@ -199,55 +200,87 @@ func (r *GetLogsRequest) UnmarshalJSON(data []byte) error {

// ========== GetTransactionByHash ============= //
type (
// Presents transacrion hash value
Copy link

Choose a reason for hiding this comment

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

transaction* spelling

Copy link
Author

Choose a reason for hiding this comment

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

Issue is resolved, see commit by hash: ac5dbdf

Nonce: "0x0",

// TODO: researching
// ? Do we need those values
Copy link

Choose a reason for hiding this comment

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

We might at a later date when it comes to recovery by signature.

// TODO: researching
// ? May be don't need this
// ! Probably, needs huge quantity requests to calc by hands
Nonce: "0x0",
Copy link

Choose a reason for hiding this comment

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

You don't need this.

Copy link
Author

Choose a reason for hiding this comment

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

Okay! In this case, we assume that this value will always be "0x0" to keep the correct value format as the graph-node requires. Issue is resolved, see commit by hash: aa08162

// TODO: discuss, consider values
// - gas limit is needed for `eth_getBlockByHash` request (return as a func result?)
// ? ethTx.GasPrice
ethTx.GasPrice = "0x0" // temporary solution
Copy link

Choose a reason for hiding this comment

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

Gas Price is generally 0x40. But I believe this should be returnable?

Copy link
Author

Choose a reason for hiding this comment

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

Issue is resolved, see commit by hash: cd94946

ethTx.To = utils.AddHexPrefix(qtumTxContractInfo.To)
ethTx.Gas = utils.AddHexPrefix(qtumTxContractInfo.GasUsed)
// TODO: discuss, consider values
// - gas limit is needed for `eth_getBlockByHash` request (return as a func result?)
Copy link

Choose a reason for hiding this comment

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

I don't believe that's the case. You should be fine with this as is.

Copy link
Author

Choose a reason for hiding this comment

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

Issue is resolved, see commit by hash: cd94946

finalOp := script[len(script)-1]
// TODO: discuss
// ? There are `witness` transactions, that is not acquireable nither via `gettransaction`, nor `getrawtransaction`
func getRewardTransactionByHash(p *qtum.Qtum, hash string) (*eth.GetTransactionByHashResponse, error) {
Copy link

Choose a reason for hiding this comment

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

why does this function exist exactly?

Copy link
Author

Choose a reason for hiding this comment

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

This function was implemented in order to serve reward transactions, such as coinbase transactions, because those transactions cannot be acquired via gettransaction method, but via getrawtransaction method.
Also implementation of this function helps keep the code a more readable and detailed by splitting the flows, despite some possible duplicates at the moment. However we may, unwrap the code of this function to keep the logic of all cases in one place.
In addition, Qtum may be improved in the future and some changes will be made to this method.

Copy link

Choose a reason for hiding this comment

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

Ok that works.

// ! Genesis block cannot be retreived
return big.NewInt(0), nil

case "pending":
Copy link

Choose a reason for hiding this comment

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

we don't really have the concept of pending when it comes to events.

Copy link
Author

Choose a reason for hiding this comment

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

Are there any developments on the way in Qtum for this concept? We think that it could become an issue in the future, if it is not included.

Copy link

Choose a reason for hiding this comment

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

There are not. If you have concerns please detail them.

@VoR0220
Copy link

VoR0220 commented Feb 8, 2021

Hey guys. Reviewing this. Just need to get the .mod and .sum conflicts resolved and we should be good to go here. Diego has retested and I think these changes get us closer rather than further from our goal.

@IntegralTeam
Copy link
Author

IntegralTeam commented Feb 12, 2021

The conflicts are resolved. Apologies for the delay.

@VoR0220 VoR0220 merged commit d41c9de into qtumproject:master Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants