thenewboston-blockchain / Bank

Bank for thenewboston digital currency.
https://thenewboston.com
MIT License
87 stars 42 forks source link

Transactions to bank's earning account are failing! #142

Closed itsnikhil closed 3 years ago

itsnikhil commented 3 years ago

Bug Description In the one of the last commit "fee": "BANK" was introduced. This change was not properly tested and I believe there is an edge case when we try to "send coins" to "bank's earning account" this fee is missing because of which transactions are failing! (see below for request and response)

Steps to Reproduce Steps to reproduce the behavior:

  1. Open 'Account manager'
  2. Click on 'Send coin'
  3. Pay to any bank's earning account; for example - 9a275161478536d0a5b88ff05d429b9a9e63d0032a46e7a6a8f088da89c69da5
  4. See error

Expected behavior

Request

Sending 500 coin from ffded318529df37bacaccc0905ad72b59d168d6024a32278421402271c0d5aba (testnet treasury) to 29865762fae7d26e51f6465b3fea436d513478cfb8aa068e88a927e887cdc5fc (testnet bank's earning account)

{
  "account_number": "ffded318529df37bacaccc0905ad72b59d168d6024a32278421402271c0d5aba",
  "message": {
    "balance_key": "ffded318529df37bacaccc0905ad72b59d168d6024a32278421402271c0d5aba",
    "txs": [
      {
        "amount": 501,
        "recipient": "29865762fae7d26e51f6465b3fea436d513478cfb8aa068e88a927e887cdc5fc"
      },
      {
        "amount": 1,
        "fee": "PRIMARY_VALIDATOR",
        "recipient": "ec8f6734272e8d9d5ea995479dd6d173424be38b313a3069d5fa62e7038a08e9"
      }
    ]
  },
  "signature": "*************************"
}

Response

Status code: HTTP 400 If you see carefully, Bank fee is missing as bank's earning account is the same!

{
  "error_message": [
    "Tx not found"
  ],
  "expected_amount": [
    "1"
  ],
  "expected_fee": [
    "BANK"
  ],
  "expected_recipient": [
    "29865762fae7d26e51f6465b3fea436d513478cfb8aa068e88a927e887cdc5fc"
  ]
}

Actual behavior We should be able to send coins to Bank's earning account so that bank can purchase confirmation services.

Screenshots image

OS and Browser

Account Number e9c5acac0806aca6ba2c0ade74d93ec4f9a89d8743fa477c52ce9b7817dcad95

itsnikhil commented 3 years ago

Will anyone acknowledge this?

jamessspanggg commented 3 years ago

@buckyroberts @angle943

angle943 commented 3 years ago

I do recall @buckyroberts bringing up this issue when we were testing for this. I forget what the conclusion was, we were either going to:

  1. fix it by adding it to the backlog, or
  2. ignore it for now until beta is out (since beta won't have this problem). Its a really edge case, so thats why it might not be worth slowing down the beta release to fix this.

@buckyroberts do you remember what we decided to do about this?

itsnikhil commented 3 years ago

"since beta won't have this problem"

How beta fixes this issue? Is it because we would merge both bank and validator?

angle943 commented 3 years ago

@itsnikhil yeah that's what I'm thinking

itsnikhil commented 3 years ago

I strongly believe that this issue would still exists if not handled. Even with bank and validator merged, "node_fee" address and receiver's address can still clash and cause the issue

angle943 commented 3 years ago

@itsnikhil i think he will address it, but just not for the alpha build

itsnikhil commented 3 years ago

@angle943 any bug bounty for reporting the issue?

angle943 commented 3 years ago

nope sorry :D even if there was a bug bounty, you are a core member, so you are ineligible for bounties haha. Your payment would be in the form of daily activity report:

"found a bug, talked with Justin about it, https://github.com/thenewboston-developers/Bank/issues/142 ~30 min or ~1 hour"