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.

Sign up for GitHub

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

Jump to bottom

Add SignedTransaction type #529

Merged
merged 19 commits into from
May 24, 2024
Merged

Add SignedTransaction type #529

merged 19 commits into from
May 24, 2024

Conversation

DOBEN
Copy link
Member

@DOBEN DOBEN commented May 2, 2024 •
edited
Loading

Purpose

Addresses Concordium/concordium-client#296
Related Concordium/concordium-client#300

Changes

Implement toJSON/fromJSON instances of the Payload types.

@DOBEN DOBEN mentioned this pull request May 2, 2024
Merged
@DOBEN DOBEN changed the title Add toJSON and fromJSON to BareBlockItemType Add SignedTransaction type May 10, 2024
@DOBEN DOBEN requested review from td202, eb-concordium and limemloh May 14, 2024 12:30
limemloh
limemloh reviewed May 14, 2024
View reviewed changes
haskell-src/Concordium/Crypto/EncryptedTransfers.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types.hs Outdated Show resolved Hide resolved
stack.yaml Outdated Show resolved Hide resolved
haskell-src/Concordium/Wasm.hs Outdated Show resolved Hide resolved
td202
td202 reviewed May 14, 2024
View reviewed changes
haskell-src/Concordium/Types.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Crypto/EncryptedTransfers.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Crypto/EncryptedTransfers.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/ID/Types.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Transactions.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Transactions.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types.hs Outdated Show resolved Hide resolved
@DOBEN DOBEN requested review from limemloh and td202 May 14, 2024 17:49
eb-concordium
eb-concordium approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@eb-concordium eb-concordium left a comment

Choose a reason for hiding this comment

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

Looks good modulo the remaining unresolved comments :)

@DOBEN DOBEN force-pushed the add-multi-sig-signing-support-in-concordium-client branch from 6c9429a to 1abaf56 Compare May 16, 2024 12:12
td202
td202 reviewed May 17, 2024
View reviewed changes
haskell-src/Concordium/Types.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Execution.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Execution.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Execution.hs Outdated Show resolved Hide resolved
haskell-tests/Types/PayloadSpec.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Transactions.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Types/Transactions.hs Outdated Show resolved Hide resolved
@DOBEN DOBEN requested a review from td202 May 21, 2024 15:11
@DOBEN DOBEN force-pushed the add-multi-sig-signing-support-in-concordium-client branch from ae8aa84 to 84de5de Compare May 21, 2024 15:26
td202
td202 approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@td202 td202 left a comment

Choose a reason for hiding this comment

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

Looks good. I would suggest adding QuickCheck tests for the JSON serializations (in the same manner as for the binary serializations) as these can potentially catch more bugs.

haskell-src/Concordium/Types/Execution.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Wasm.hs Outdated Show resolved Hide resolved
haskell-src/Concordium/Wasm.hs Outdated Show resolved Hide resolved
@DOBEN DOBEN mentioned this pull request May 24, 2024
Open
@DOBEN
Copy link
Member Author

DOBEN commented May 24, 2024

For now, I created an issue to improve on the tests:

#535

@DOBEN DOBEN merged commit 1c97f30 into main May 24, 2024
40 checks passed
@DOBEN DOBEN deleted the add-multi-sig-signing-support-in-concordium-client branch May 24, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@td202 td202 td202 approved these changes

@eb-concordium eb-concordium Awaiting requested review from eb-concordium

@limemloh limemloh Awaiting requested review from limemloh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DOBEN @td202 @limemloh @eb-concordium