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

Bug fix for FastRawTransactionManager.resetNonce #2084

Merged
merged 2 commits into from
Aug 10, 2024

Conversation

junsung-cho
Copy link
Contributor

@junsung-cho junsung-cho commented Jul 25, 2024

What does this PR do?

Make FastRawTransactionManager.resetNonce() reset the nonce

Where should the reviewer start?

#2002

Why is it needed?

AS-IS

    public synchronized void resetNonce() throws IOException {
        nonce = super.getNonce();
    }

TO-BE

    public synchronized void resetNonce() throws IOException {
        nonce = BigInteger.valueOf(-1);
    }

While the AS-IS reset nonce with the current nonce, if getNonce() is called, it will return currentNonce + 1.

    @Override
    protected synchronized BigInteger getNonce() throws IOException {
        if (nonce.signum() == -1) {
            // obtain lock
            nonce = super.getNonce();
        } else {
            nonce = nonce.add(BigInteger.ONE); // this line will be called
        }
        return nonce;
    }

Checklist

  • I've read the contribution guidelines.
  • I've added tests (if applicable).
  • I've added a changelog entry if necessary.

fix #2002

@gtebrean
Copy link
Contributor

@junsung-cho Looks good to me, please also update the changelog.md and I will merge it

@junsung-cho
Copy link
Contributor Author

@gtebrean I updated CHANGELOG.md, thanks

@junsung-cho
Copy link
Contributor Author

@gtebrean Integration test failed, but I don't know why.

BesuPrivacyQuickstartIntegrationTest > simplePrivateTransactions() FAILED
    org.opentest4j.AssertionFailedError at BesuPrivacyQuickstartIntegrationTest.java:193

@NickSneo
Copy link
Contributor

@gtebrean Integration test failed, but I don't know why.

BesuPrivacyQuickstartIntegrationTest > simplePrivateTransactions() FAILED
    org.opentest4j.AssertionFailedError at BesuPrivacyQuickstartIntegrationTest.java:193

It is a bit flaky test, I will rerun it

@junsung-cho junsung-cho force-pushed the fix-reset-nonce branch 5 times, most recently from 3add0b5 to aeba374 Compare August 9, 2024 09:06
Signed-off-by: Junsung Cho <[email protected]>
@NickSneo NickSneo merged commit 7e3465b into hyperledger:main Aug 10, 2024
5 checks passed
@junsung-cho junsung-cho deleted the fix-reset-nonce branch August 11, 2024 10:59
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.

FastRawTransactionManager's resetNonce() will bring its nonce field out of sync
3 participants