TopUp amount is not credited / incorrectly credited to stamps

[swarmHaseeb]

⚠️ Support requests in an issue-format will be closed immediately. For support, go to Swarm's Discord.

Summary

TopUp amount is not credited to stamp because the functionality does not exist in HandleTopUp func in mock/service. TopUp amount is also calculated incorrectly from normalized balance.

Expected behavior

/stamps/topup{id}/{amount} should add the credited amount to stamp

Actual behavior

Topup is not credited to stamp as seen in the screenshot below. Topup should add the amount to the specific stamp but the amount does not change.

If a fix is applied, we get another bug, that is incorrect amount which is added to the amount and can be seen after api call. This incorrect amount is displayed because of the normalized balance received from smart contract which needs to be divided by 2^depth to get the actual amount. This behavior was also verified by @istae and @acud in production code.

Steps to reproduce

Please use the following commands on master branch to reproduce the issue. curl localhost:1635/stamps curl -XPOST localhost:1635/stamps/1000/20 curl -XPATCH localhost:1635/stamps/topup/{stampID}/1000 curl localhost:1635/stamps

To verify incorrect topup, you can modify and add the HandleTopUp function in mock/service with

        for _, v := range m.issuersMap {
		if bytes.Equal(batchID, v.ID()) {
			if newBalance.Cmp(v.Amount()) > 0 {
				v.Amount().Set(newBalance)
			}
			return
		}
	}

Possible solution

A possible solution for this issue is to correctly calculate the topup amount from normalized balance using normalizedBalance / 2^Depth and adding the functionality in HandleTopUp in mock/service.

[ghost]

@agazso @istae for kind notice.

[aloknerurkar]

So the name of the package is mock/service. Anything that is part of mock is only used in the tests or debug side. It is not a user facing functionality. Which is why this is not implemented. The actual implementation of this, which is used, is in the postage package.

This is not something that we support. The dev mode has been defined as a quick way for users to test API. The postage related operations are meant to change state on the blockchain as a result testing them in dev mode is not required in my opinion. The goal of dev mode is to just show how the output would look. Users should not write code based on this output as this is undefined behaviour when you are not connected to an actual blockchain.

If at all something like this is required, we should be implementing a separate package for this with unit tests. Right now the mock classes are not unit tested as they are not meant to function. They just provide placeholders in tests. If there is more functionality in them, they need to be unit tested.

[ghost]

@aloknerurkar If a fix is applied, we get another bug, that is incorrect amount which is added to the amount and can be seen after api call. This incorrect amount is displayed because of the normalized balance received from smart contract which needs to be divided by 2^depth to get the actual amount.

This issue related to normalized balance can also be found at batchservice and postage/service and @istae verified that this problem doesn't just exist in dev mode.

[Cafe137]

Bee 1.7.0 mainnet

First I buy a stamp with amount 100.000

swarm-cli stamp buy --depth 22 --amount 100k --label test
Stamp ID: 05e69442c78bf6bb894b09decf148d9b14d82b4faf2e28d6e02b2afde884ef7c

Print it

swarm-cli stamp show -v 05e69442c78bf6bb894b09decf148d9b14d82b4faf2e28d6e02b2afde884ef7c
Amount: 100000

Then topup with 100.000 again

swarm-cli stamp topup --stamp 05e69442c78bf6bb894b09decf148d9b14d82b4faf2e28d6e02b2afde884ef7c --amount 100k

It reports back 100.000, but after a few minutes the new amount appears, however it is incorrect.

swarm-cli stamp show -v 05e69442c78bf6bb894b09decf148d9b14d82b4faf2e28d6e02b2afde884ef7c                    
Amount: 28350016

Expected 200.000, actual 28.350.016