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

Add user nonce to TransactionOptions type #244

Merged
merged 5 commits into from
Aug 25, 2022
Merged

Conversation

germartinez
Copy link
Member

What it solves

Allows to specify the nonce from the sender wallet when executing a Safe transaction via the Core SDK.

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@@ -53,6 +53,10 @@ class Web3Adapter implements EthAdapter {
return BigNumber.from(await this.#web3.eth.getBalance(address))
}

async getNonce(address: string): Promise<number> {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to proxy this function? If we really do, please also add a second optional param called blockTag.

Copy link
Member

Choose a reason for hiding this comment

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

What can the blockTag be used for?

Copy link
Member

Choose a reason for hiding this comment

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

In the app, we're passing pending to fetch unfinished txen, because that's how you know the latest nonce.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, we should just probably pass pending here, like in safe-react, w/o making it a parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

getNonce is not used directly when using the user nonce from the TransactionOptions type.
It is used in tests and it could also be useful for people playing around with the nonce property.

I added an optional argument that allows to specify the blockTag, adding it to methods that were not receiving it as well.

Copy link
Member

@usame-algan usame-algan left a comment

Choose a reason for hiding this comment

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

🚀

@germartinez germartinez merged commit 35ab59c into development Aug 25, 2022
@germartinez germartinez deleted the add-user-nonce branch August 25, 2022 15:08
@github-actions github-actions bot locked and limited conversation to collaborators Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants