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

chore::> Introduce common account.SendTransaction function to dynamically call Add<Type>Transaction. #592

Merged
merged 12 commits into from
Oct 3, 2024
Merged

chore::> Introduce common account.SendTransaction function to dynamically call Add<Type>Transaction. #592

merged 12 commits into from
Oct 3, 2024

Conversation

PsychoPunkSage
Copy link
Contributor

Addresses #590 pointer 3

@PsychoPunkSage PsychoPunkSage marked this pull request as draft July 11, 2024 12:51
@PsychoPunkSage PsychoPunkSage marked this pull request as ready for review July 11, 2024 13:12
thiagodeev
thiagodeev requested changes Jul 11, 2024
View reviewed changes
Copy link
Collaborator

@thiagodeev thiagodeev left a comment

Choose a reason for hiding this comment

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

Nice @PsychoPunkSage.
I left some comments

account/account.go Outdated Show resolved Hide resolved
account/account.go Outdated Show resolved Hide resolved
account/account.go Outdated Show resolved Hide resolved
account/account.go Outdated Show resolved Hide resolved
account/account.go Outdated Show resolved Hide resolved
examples/deployAccount/main.go Outdated Show resolved Hide resolved
account/account_test.go Outdated Show resolved Hide resolved
@PsychoPunkSage
Copy link
Contributor Author

Hey @thiagodeev
Some documentation changes are left...
Rest everything is done..
Have a look at it.

thiagodeev
thiagodeev requested changes Jul 16, 2024
View reviewed changes
Copy link
Collaborator

@thiagodeev thiagodeev left a comment

Choose a reason for hiding this comment

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

Great @PsychoPunkSage! We're almost there.
Left a few comments

account/account.go Outdated Show resolved Hide resolved
rpc/types_transaction_response.go Outdated Show resolved Hide resolved
@PsychoPunkSage
Copy link
Contributor Author

@thiagodeev
I have made required changes...
But for now we shouldn't remove AddInvokeTransactionResponse, AddDeployAccountTransactionResponse, and the AddInvokeTransactionResponse type cause it is used in multiple places...
Plus I don't want this PR to get any longer for now :)
What do you think?

thiagodeev
thiagodeev requested changes Jul 17, 2024
View reviewed changes
rpc/types_transaction_response.go Outdated Show resolved Hide resolved
@thiagodeev
Copy link
Collaborator

Okay @PsychoPunkSage, I agree with you. Since we have these TxResponse being used by provider methods, let's leave them for another time ;)

@PsychoPunkSage
Copy link
Contributor Author

Done @thiagodeev

thiagodeev
thiagodeev requested changes Jul 18, 2024
View reviewed changes
Copy link
Collaborator

@thiagodeev thiagodeev left a comment

Choose a reason for hiding this comment

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

It's almost completed!
Just one thing before merging:

account/account_test.go Show resolved Hide resolved
thiagodeev
thiagodeev approved these changes Jul 22, 2024
View reviewed changes
Copy link
Collaborator

@thiagodeev thiagodeev left a comment

Choose a reason for hiding this comment

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

Great @PsychoPunkSage! LGTM

@thiagodeev thiagodeev changed the title chore::> Introduce common account.AddTransaction function to dynamically call Add<Type>Transaction. chore::> Introduce common account.SendTransaction function to dynamically call Add<Type>Transaction. Jul 22, 2024
@PsychoPunkSage
Copy link
Contributor Author

Hi @rianhughes @thiagodeev
I think this PR is ready tobe merged

@thiagodeev thiagodeev changed the base branch from main to v0.8.0 October 3, 2024 15:35
@thiagodeev thiagodeev changed the base branch from v0.8.0 to main October 3, 2024 15:36
@thiagodeev thiagodeev changed the base branch from main to v0.8.0 October 3, 2024 17:13
@thiagodeev thiagodeev merged commit 9cb1fb9 into NethermindEth:v0.8.0 Oct 3, 2024
4 checks passed
@thiagodeev thiagodeev added this to the v0.8.0 milestone Oct 3, 2024
This was referenced Dec 4, 2024
Closed
Merged
thiagodeev added a commit that referenced this pull request Dec 11, 2024
@thiagodeev @PsychoPunkSage @rianhughes
Merge PR #592 to main (#647)
1c399f4 
* fix::> introduced comman AddTransaction function in account package

* fix::> replace all the instances of Add<Type>Transaction in example/test with AddTransaction

* fix::> func name and doc changes made

* fix::> account_test.go changed

* fix::> examples error solved

* fix::> Removed type assertions, created generic TransactionResponse struct, and transaction type converter

* fix::> removed unnecessary lines

* fix:> made required changes

* fix::> removed unnecessary comments

* fix::> renamed test function

---------

Co-authored-by: Abhinav Prakash <[email protected]>
Co-authored-by: Rian Hughes <[email protected]>
thiagodeev added a commit that referenced this pull request Dec 19, 2024
@thiagodeev @PsychoPunkSage @rianhughes
Merge PR #592 to main (#647)
6b5fc7f 
* fix::> introduced comman AddTransaction function in account package

* fix::> replace all the instances of Add<Type>Transaction in example/test with AddTransaction

* fix::> func name and doc changes made

* fix::> account_test.go changed

* fix::> examples error solved

* fix::> Removed type assertions, created generic TransactionResponse struct, and transaction type converter

* fix::> removed unnecessary lines

* fix:> made required changes

* fix::> removed unnecessary comments

* fix::> renamed test function

---------

Co-authored-by: Abhinav Prakash <[email protected]>
Co-authored-by: Rian Hughes <[email protected]>
thiagodeev added a commit that referenced this pull request Dec 24, 2024
@thiagodeev
Squashed commit of the following:
59ff5c7 
commit 9cb940b
Author: Thiago Ribeiro <[email protected]>
Date:   Thu Dec 19 23:10:28 2024 -0300

    Thiagodeev/rpcv08 final changes (#644)

    * Adjusts TransactionStatus

    * Adjusts EstimateFee

    * Fixes blockTest error

    * Adds GetStorageProof method (#635)

    * implement rpcv8 GetStorageProof method

    * Adds GetMessagesStatus method (#634)

    * Added GetMessagesStatus method

    * PR comment fixes

    * Fix MerkleNode type

    * Updates PendingBlockReader type

    * Updates ExecutionResources and remove ComputationResources

    * Fixes failing tests and improves Error() resp

    * Adds TODO comments and set a pointer to Marshal method

    * Thiagodeev/rpcv08 write methods, blockHeader and error 53 (#626)

    Update write methods, error 53 and block header for rpc08

    * Adjusts EstimateFee

commit 4ca79eb
Merge: 9007483 8ecf779
Author: Thiago Ribeiro <[email protected]>
Date:   Wed Dec 18 00:35:32 2024 -0300

    Merge branch 'main' into v0.8.0

commit 8ecf779
Author: Thiago Ribeiro <[email protected]>
Date:   Fri Dec 13 10:30:35 2024 -0300

    Implements SNIP-12 (#637)

    * General semantic updates and EncodeType adjustment

    * Improvements and bug fixes

    * Implements typedData unmarshal

    * Adds Message support on Unmarshal typedData

    * Rename files and create types file

    * Creates TestGeneral_CreateMessageWithTypes

    * Clears some curve methods and adds Poseidon method

    * Adds revision.go file, new Revision field of TypedData

    * Basic implementation working with a rev 0 typedData

    * Rename files

    * Some code adjustments

    * Adds new examples from starknet.js and implements new mock logic

    * init function on revision package

    * Adds first version of Validate feature

    * Adds revision 1 support to encodeType

    * Adds 'selector' support in encodeData

    * restructs encodeData

    * adds handleStandardTypes and handleArrays functions

    * support to arrays, new types and new Domain unmarshal

    * fixed support to bool, new example being tested

    * fixes errors in encodeType, 'example_presetTypes' supported

    * implements merkletree encode

    * adds support to 'mail_StructArray' json example

    * Fixes error inStringToByteArrFelt func, 'v1Nested' example passing

    * Started to refactor 'encodeType' func

    * Fixes bug with merkletree

    * Creates the verifyType func

    * implements enum encoding

    * Removes the validation method, it will be added later

    * creates big example for testing purpose

    * removes the types file

    * adds code comments and descriptions

    * rename folder and file names

    * creates typedData example and change READMEs

    * Update utils/Felt.go

    Co-authored-by: Rian Hughes <[email protected]>

    * Update utils/keccak.go

    Co-authored-by: Rian Hughes <[email protected]>

    * addresses Rian's comment about GetMessageHash

    * Revert enum wrong encode as it was fixed by starknet.js

    * Creates ValidationSignature helper

    * Removes the StrToFelt utility

    * Improves 'chainId' validation

    * Changes private fields of TypedData to public

    ---------

    Co-authored-by: Rian Hughes <[email protected]>

commit 1c399f4
Author: Thiago Ribeiro <[email protected]>
Date:   Wed Dec 11 12:13:13 2024 -0300

    Merge PR #592 to main (#647)

    * fix::> introduced comman AddTransaction function in account package

    * fix::> replace all the instances of Add<Type>Transaction in example/test with AddTransaction

    * fix::> func name and doc changes made

    * fix::> account_test.go changed

    * fix::> examples error solved

    * fix::> Removed type assertions, created generic TransactionResponse struct, and transaction type converter

    * fix::> removed unnecessary lines

    * fix:> made required changes

    * fix::> removed unnecessary comments

    * fix::> renamed test function

    ---------

    Co-authored-by: Abhinav Prakash <[email protected]>
    Co-authored-by: Rian Hughes <[email protected]>

commit 9007483
Author: Thiago Ribeiro <[email protected]>
Date:   Wed Nov 6 11:26:01 2024 -0300

    starknet_getCompiledCasm (#642)

    * implement starknet_getCompiledCasm for RPCv8

commit 98d2b5c
Author: Thiago Ribeiro <[email protected]>
Date:   Mon Oct 28 11:24:53 2024 -0300

    Adds GetMessagesStatus method (#634)

    * Added GetMessagesStatus method

    * PR comment fixes

commit ae6e233
Author: Thiago Ribeiro <[email protected]>
Date:   Mon Oct 28 07:20:01 2024 -0300

    Adds GetStorageProof method (#635)

    * implement rpcv8 GetStorageProof method

commit 75425d3
Author: thiagodeev <[email protected]>
Date:   Mon Sep 30 10:08:19 2024 -0300

    Fixes blockTest error

commit de722c1
Author: thiagodeev <[email protected]>
Date:   Mon Sep 30 10:07:56 2024 -0300

    Adjusts EstimateFee

commit b97a149
Author: thiagodeev <[email protected]>
Date:   Mon Sep 30 09:31:31 2024 -0300

    Adjusts TransactionStatus
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thiagodeev thiagodeev thiagodeev approved these changes

@rianhughes rianhughes Awaiting requested review from rianhughes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.
3 participants
@PsychoPunkSage @thiagodeev @rianhughes