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

There is a problem with the change processing logic #98

Closed
Java1024 opened this issue Oct 22, 2019 · 4 comments
Closed

There is a problem with the change processing logic #98

Java1024 opened this issue Oct 22, 2019 · 4 comments
Labels
bug

Comments

@Java1024
Copy link

@Java1024 Java1024 commented Oct 22, 2019

private void calculateInputsAndChange(Map<String, BigDecimal> requiredAssets, Account
changeAcct) {

        requiredAssets.forEach((assetId, requiredAmount) -> {
            List<Utxo> selectedUtxos = this.inputCalculationStrategy.calculateInputs(
                    this.utxos.get(assetId), requiredAmount);

            this.inputs.addAll(selectedUtxos.stream()
                    .map(Utxo::toTransactionInput)
                    .collect(Collectors.toList()));

            this.outputs.add(getChangeTransactionOutput(assetId, requiredAmount,
                    selectedUtxos, changeAcct.getAddress()));
        });
    }

    private RawTransactionOutput getChangeTransactionOutput(String assetId,
                                                            BigDecimal requiredValue,
                                                            List<Utxo> utxos,
                                                            String changeAddress) {

        BigDecimal inputAmount = utxos.stream().map(Utxo::getValue).reduce(BigDecimal::add).get();
        if (inputAmount.compareTo(requiredValue) <= 0) {
            return null;
        }
        BigDecimal change = inputAmount.subtract(requiredValue);
        return new RawTransactionOutput(assetId, change.toPlainString(), changeAddress);
    }

Output will add a null if all assets are not removed, causing a java.lang.NullPointerException to occur with writeSerializableFixed()

@gsmachado

This comment has been minimized.

Copy link
Member

@gsmachado gsmachado commented Oct 22, 2019

@Java1024 thanks for your report! We'll definitely look into it.
Can you please provide us a reproduction case?

@gsmachado gsmachado added the bug label Oct 22, 2019
@Java1024

This comment has been minimized.

Copy link
Author

@Java1024 Java1024 commented Oct 22, 2019

Neow3j neow3j = Neow3j.build(new HttpService("http://seed7.ngd.network:10332"));
Account multiSigAcct = Account.fromAddress("ATcWffQV1A7NMEsqQ1RmKfS7AbSqcAp2hd").build();
multiSigAcct.updateAssetBalances(neow3j);
AssetTransfer at = new AssetTransfer.Builder(neow3j)
.account(multiSigAcct)
.output(NEOAsset.HASH_ID, 1, "AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y")
.build();

byte[] unsignedTxHex = at.getTransaction().toArrayWithoutScripts();
SignatureData sig1 = Sign.signMessage(unsignedTxHex, keyPair1);
SignatureData sig2 = Sign.signMessage(unsignedTxHex, keyPair2);
RawScript witness = RawScript.createMultiSigWitness(2, Arrays.asList(sig1, sig2), keys);
at.addWitness(witness).send();

As with the documentation, executing byte[] unsignedTxHex = at.getTransaction().toArrayWithoutScripts(); will report java.lang.NullPointerException

csmuller added a commit that referenced this issue Oct 22, 2019
csmuller added a commit that referenced this issue Oct 22, 2019
@csmuller

This comment has been minimized.

Copy link
Member

@csmuller csmuller commented Oct 22, 2019

Fixed the issue. Though, release date of next neow3j patch version is not set yet.

gsmachado added a commit that referenced this issue Nov 3, 2019
* develop-2.x:
  Add new logos
  #98: Fix bug in AssetTransfer.
gsmachado added a commit that referenced this issue Dec 8, 2019
* develop-2.x: (237 commits)
  Change the README logo
  Add new logos
  #98: Fix bug in AssetTransfer.
  Correct possible null pointer issue
  Increment version number to 2.3.0
  Remove redundant test
  Remove the network fee check
  Move network fee calculation method to utils
  Fix bug in ByteArrayStackItem
  Fix Travis build errors
  Fix bug in Builder method, correct and add docu
  Update README.md
  Update Android statistics
  Add multi-sig address test to asset transfer
  Use doubles and strings instead of BigDecimals
  #90: Add javadoc and other minor corrections
  Attempt to fix Travis build errors
  #90: Fix Codacy issue
  #90: Add tests
  #90: Make minor amendments
  ...

# Conflicts:
#	build.gradle
#	core/build.gradle
#	core/src/main/java/io/neow3j/protocol/Service.java
#	core/src/main/java/io/neow3j/protocol/core/JsonRpc2_0Neow3j.java
gsmachado added a commit that referenced this issue Dec 8, 2019
* develop-2.x: (82 commits)
  Change the README logo
  Add new logos
  #98: Fix bug in AssetTransfer.
  Correct possible null pointer issue
  Increment version number to 2.3.0
  Remove redundant test
  Remove the network fee check
  Move network fee calculation method to utils
  Fix bug in ByteArrayStackItem
  Fix Travis build errors
  Fix bug in Builder method, correct and add docu
  Update README.md
  Update Android statistics
  Add multi-sig address test to asset transfer
  Use doubles and strings instead of BigDecimals
  #90: Add javadoc and other minor corrections
  Attempt to fix Travis build errors
  #90: Fix Codacy issue
  #90: Add tests
  #90: Make minor amendments
  ...
gsmachado added a commit that referenced this issue Dec 12, 2019
* develop-2.x:
  Update version for release
  Change the README logo
  Add new logos
  #98: Fix bug in AssetTransfer.
  Correct possible null pointer issue
@gsmachado

This comment has been minimized.

Copy link
Member

@gsmachado gsmachado commented Dec 30, 2019

Released on 2.3.1.

@gsmachado gsmachado closed this Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.