nFeePaid can be overflow. #76

New issue

Closed

opened 2020-10-06 00:02:49 +00:00 by cryptcoin-junkey · 3 comments

cryptcoin-junkey commented

2020-10-06 00:02:49 +00:00

(Migrated from github.com)

From @wakiyamap https://github.com/monacoinproject/monacoin/pull/72#discussion_r458445040

If value of nFeePaid is 10512000000000000(MAX_MONEY), it's overflow?
nFeePaid * 1000 =
10,512,000,000,000,000,000
int64_t of max is
9,223,372,036,854,775,807

So......how about adding the following?
assert(nFeePaid<= MAX_MONEY);

Personaly, I'd like to believe that maybe this isn't the case.

From @wakiyamap https://github.com/monacoinproject/monacoin/pull/72#discussion_r458445040 > If value of nFeePaid is 10512000000000000(MAX_MONEY), it's overflow? > nFeePaid * 1000 = > 10,512,000,000,000,000,000 > int64_t of max is > 9,223,372,036,854,775,807 > > So......how about adding the following? > assert(nFeePaid<= MAX_MONEY); > > Personaly, I'd like to believe that maybe this isn't the case.

cryptcoin-junkey commented

2020-10-06 06:46:30 +00:00

(Migrated from github.com)

I suggested code like this.

nSatoshisPerK = (CAmount)((__int128_t)nFeePaid * 1000 / nSize);

But It will still overflow in case nFeePaid == INT64_MAX and nSize < 1000.
So my next plan is.

__int128_t n = (__int128_t)nFeePaid * 1000 / nSize;
assert(n <= INT64_MAX);
nSatoshisPerK = (CAmount)n;

I suspect this issue is a nit pick. But I can send PR if required.
@wakiyamap @pendingbot

I suggested code like this. ``` nSatoshisPerK = (CAmount)((__int128_t)nFeePaid * 1000 / nSize); ``` But It will still overflow in case nFeePaid == INT64_MAX and nSize < 1000. So my next plan is. ``` __int128_t n = (__int128_t)nFeePaid * 1000 / nSize; assert(n <= INT64_MAX); nSatoshisPerK = (CAmount)n; ``` I suspect this issue is a nit pick. But I can send PR if required. @wakiyamap @pendingbot

👍 2

cryptcoin-junkey commented

2020-10-06 07:02:05 +00:00

(Migrated from github.com)

Test snippet.

#include <stdio.h>
#include <stdint.h>
#include <inttypes.h>
#include <assert.h>

typedef int64_t CAmount;

int
main(void)
{
  CAmount nFeePaid = INT64_MAX;
  CAmount nSatoshisPerK;

  setbuf(stdout, NULL);
  for (size_t nSize = 1002; ; nSize--) {
    printf("%zu %" PRId64 , nSize, nFeePaid);
    __int128_t n = (__int128_t)nFeePaid * 1000 / nSize;
    assert(n <= INT64_MAX);
    nSatoshisPerK = (CAmount)n;
    printf(" %" PRId64 "\n", nSatoshisPerK);
    fflush(stdout);
  }
  return 0;
}

The result on my terminal.

./a.out
1002 9223372036854775807 9204962112629516773
1001 9223372036854775807 9214157878975800006
1000 9223372036854775807 9223372036854775807
999 9223372036854775807a.out: main.c:18: main: Assertion `n <= INT64_MAX' failed.
Aborted (core dumped)

Test snippet. ``` #include <stdio.h> #include <stdint.h> #include <inttypes.h> #include <assert.h> typedef int64_t CAmount; int main(void) { CAmount nFeePaid = INT64_MAX; CAmount nSatoshisPerK; setbuf(stdout, NULL); for (size_t nSize = 1002; ; nSize--) { printf("%zu %" PRId64 , nSize, nFeePaid); __int128_t n = (__int128_t)nFeePaid * 1000 / nSize; assert(n <= INT64_MAX); nSatoshisPerK = (CAmount)n; printf(" %" PRId64 "\n", nSatoshisPerK); fflush(stdout); } return 0; } ``` The result on my terminal. ``` ./a.out 1002 9223372036854775807 9204962112629516773 1001 9223372036854775807 9214157878975800006 1000 9223372036854775807 9223372036854775807 999 9223372036854775807a.out: main.c:18: main: Assertion `n <= INT64_MAX' failed. Aborted (core dumped) ```

cryptcoin-junkey commented

2021-01-07 05:57:45 +00:00

(Migrated from github.com)

Fixed by 31d7ea3.

Fixed by 31d7ea3.