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

docs: create commonTransactionParameters to hold parameters that are common between all transaction types #209

Conversation

rwalworth
Copy link
Contributor

@rwalworth rwalworth commented May 15, 2024

- Update signerKey to a list that will allow for multiple additional signatures of a transaction
- Explicitly use signerKeys to provide any additional signatures to a transaction. The transaction will not be signed (except by the fee payer) if signerKeys is empty.

While working on #216, I noticed an issue that would need to be addressed at some point: common parameters for all transactions. I decided to hijack this PR with a proposed solution since it was already addressing an issue that was tangent to it. Basically in order to allow ourselves to test all fields for a transaction (most notably, the parameters that are common to all transactions), we would need to add all of these parameters

This PR now adds the specification for an additional JSON parameter object that can be shared between transactions, commonTransactionParams. This will allow every transaction to hold common parameters in one defined object, as well as hold signatures (which was this PR's original intent). signerKey is now signers, and is a list contained in commonTransactionParams. The transaction will now not be signed (except by the fee payer established with the setOperator method) if commonTransactionParams.signers is empty.

Related issue(s):

Fixes #208

@rwalworth rwalworth added the enhancement New feature or request label May 15, 2024
@rwalworth rwalworth self-assigned this May 15, 2024
@rwalworth rwalworth linked an issue May 15, 2024 that may be closed by this pull request
@rwalworth rwalworth changed the title docs: use signerKeys to explicitly state when additional keys should sign a transaction docs: use signerKeys to explicitly state when additional keys should sign an AccountCreateTransaction May 15, 2024
@rwalworth rwalworth force-pushed the 00208-use-signerkey-as-a-way-to-ensure-additional-signatures-in-accountcreatetransaction branch from 0dde811 to 0e65def Compare May 15, 2024 17:58
@rwalworth rwalworth requested a review from thenswan May 15, 2024 20:29
…_PRIVATE_KEY or VALID_PUBLIC_KEY

Signed-off-by: Rob Walworth <[email protected]>
Copy link

@thenswan thenswan left a comment

Choose a reason for hiding this comment

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

…t are common between all transactions

Signed-off-by: Rob Walworth <[email protected]>
@rwalworth rwalworth changed the title docs: use signerKeys to explicitly state when additional keys should sign an AccountCreateTransaction docs: create 'commonTransactionParameters` to hold parameters that are common between all transaction types May 22, 2024
@rwalworth rwalworth changed the title docs: create 'commonTransactionParameters` to hold parameters that are common between all transaction types docs: create commonTransactionParameters to hold parameters that are common between all transaction types May 22, 2024
@rwalworth rwalworth requested a review from thenswan May 22, 2024 18:48
@rwalworth rwalworth added documentation Improvements or additions to documentation and removed enhancement New feature or request labels May 22, 2024
rwalworth added a commit that referenced this pull request May 23, 2024
Copy link

@thenswan thenswan left a comment

Choose a reason for hiding this comment

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

I guess it makes sense, although would like to have a review here from other team members as well.

@thenswan
Copy link

@rwalworth will AccountCreateTransaction be updated with commonTransactionParams?

@rwalworth
Copy link
Contributor Author

rwalworth commented May 28, 2024

@rwalworth will AccountCreateTransaction be updated with commonTransactionParams?

Yes, the createAccount JSON-RPC method will be updated to contain this (#205 and #206)

@rwalworth rwalworth merged commit df50d5c into main May 29, 2024
5 checks passed
@rwalworth rwalworth deleted the 00208-use-signerkey-as-a-way-to-ensure-additional-signatures-in-accountcreatetransaction branch May 29, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation for a common JSON transaction parameters object
2 participants