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

chore: cleanup internals #329

Merged
merged 7 commits into from Nov 27, 2023
Merged

chore: cleanup internals #329

merged 7 commits into from Nov 27, 2023

Conversation

shariffdev
Copy link
Contributor

@shariffdev shariffdev commented Oct 17, 2023

Also resolving #153

@shariffdev shariffdev changed the title chore: cleanup Signer chore: cleanup internals Oct 18, 2023
@shariffdev shariffdev marked this pull request as ready for review October 18, 2023 07:50
actions.clone(),
block_hash,
),
signed_transaction(signer, actions.clone(), nonce, block_hash, receiver_id),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer constructor method (from_actions or maybe other name) on SignedTransaction over a standalone function as it is harder to discover the function. What is exactly the motivation behind this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function is a reimplementation of from_actions without requiring Signer trait as an argument. The hint I took here is that we can condense that implementation and remove the reliance on the trait.
I could name it better.

Comment on lines 542 to 545
SignedTransaction::new(
signer.secret_key.0.sign(tx.get_hash_and_size().0.as_ref()),
tx,
)
Copy link
Member

Choose a reason for hiding this comment

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

you shouldn't need to call into SignedTransaction::new. A tx.sign(&signer) is simpler

@frol frol merged commit a6126ec into near:main Nov 27, 2023
6 checks passed
@frol frol mentioned this pull request Nov 27, 2023
@shariffdev shariffdev deleted the chore/cleanup-signer branch November 28, 2023 13:43
frol added a commit that referenced this pull request Jan 25, 2024
## 🤖 New release
* `near-workspaces`: 0.9.0 -> 0.10.0 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.10.0](near-workspaces-v0.9.0...near-workspaces-v0.10.0)
- 2024-01-25

### Other
- Impl Clone on result Value
([#345](#345))
- Upgraded NEAR crates to 0.20.0 release
([#346](#346))
- dependecy bumps
([#338](#338))
- cleanup internals
([#329](#329))
- use stable sandbox by default
([#335](#335))
- [**breaking**] Remove `interop_sdk` feature from defaults
([#339](#339))
- fix typos
([#340](#340))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Signed-off-by: Vlad <304265+frol@users.noreply.github.com>
Co-authored-by: Vlad <304265+frol@users.noreply.github.com>
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.

None yet

3 participants