-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(jsonrpc): add "input" as field for transaction input #6467
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,9 @@ public class CallArguments { | |
| @Getter | ||
| @Setter | ||
| private String nonce; // not used | ||
| @Getter | ||
| @Setter | ||
| private String input; //Add input parameter to align with Ethereum https://github.com/ethereum/go-ethereum/pull/28078 | ||
|
|
||
| /** | ||
| * just support TransferContract, CreateSmartContract and TriggerSmartContract | ||
|
|
@@ -57,8 +60,11 @@ public ContractType getContractType(Wallet wallet) throws JsonRpcInvalidRequestE | |
| throw new JsonRpcInvalidRequestException("invalid json request"); | ||
| } else if (paramStringIsNull(to)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new Those are not equivalent in this codebase because That creates an inconsistent flow where the request can be validated based on Concrete example:
Please use one canonical transaction-data resolver and one emptiness definition everywhere. |
||
| // data is null | ||
| if (paramStringIsNull(data)) { | ||
| throw new JsonRpcInvalidRequestException("invalid json request"); | ||
| if (paramStringIsNull(input)) { | ||
| if(paramStringIsNull(data)) | ||
| { | ||
| throw new JsonRpcInvalidRequestException("invalid json request"); //If both input and data are bnull throw an error, take input as priority | ||
| } | ||
| } | ||
|
|
||
| contractType = ContractType.CreateSmartContract; | ||
|
|
@@ -86,4 +92,26 @@ public ContractType getContractType(Wallet wallet) throws JsonRpcInvalidRequestE | |
| public long parseValue() throws JsonRpcInvalidParamsException { | ||
| return parseQuantityValue(value); | ||
| } | ||
|
|
||
| /** | ||
| * Get the transaction data, supporting both 'data' and 'input' parameters | ||
| * for Ethereum compatibility. 'input' takes precedence if both are provided. | ||
| * | ||
| * @return the transaction data/input | ||
| */ | ||
| public String getTransactionData() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The If both Please add a shared conflict check for:
and return a clear params error instead of silently swallowing one side. |
||
| // Prioritize 'input' for Ethereum compatibility | ||
| if (StringUtils.isNotEmpty(input)) { | ||
| return input; | ||
| } | ||
| return data; | ||
| } | ||
|
|
||
| /** | ||
| * Set transaction data, updating the appropriate field based on which was used | ||
| */ | ||
| public void setTransactionData(String transactionData) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is a maintainability trap for future callers. Please either remove it, or document very explicitly why it only updates |
||
| // For backward compatibility, default to setting 'data' | ||
| this.data = transactionData; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ public class CallArgumentsTest extends BaseTest { | |
| public void init() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has the same problem as |
||
| callArguments = new CallArguments("0x0000000000000000000000000000000000000000", | ||
| "0x0000000000000000000000000000000000000001","0x10","0.01","0x100", | ||
| "","0"); | ||
| "","0",""); | ||
| } | ||
|
|
||
| @Test | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These helpers(
paramTransactionDataIsNullandgetTransactionData) look like the intended canonical implementation, but they are not actually used byCallArguments/BuildArguments.Right now the PR adds a shared utility and still keeps separate local implementations, and those implementations already differ on
"0x"semantics. Please either:but avoid leaving the refactor half-done.