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

implement transaction reversion #61

Merged
merged 15 commits into from
Oct 28, 2021

Conversation

henry-jackson
Copy link
Contributor

Signed-off-by: Henry Jackson henry.jackson.95@gmail.com

Closes #52

▶ curl -X POST -H "Content-Type: application/json" http://localhost:3068/test-01/transactions -d \                                                           
'{
   "postings": [
     {
       "source": "world",
       "destination": "test:001",
       "amount": 100,
       "asset": "USD/2"
     }
   ],
   "reference": "foobar"
 }'

{"ok":true}%                                                                                                                                                  

▶ curl -X GET http://localhost:3068/test-01/transactions | jq .                                                                                              
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   352  100   352    0     0   171k      0 --:--:-- --:--:-- --:--:--  171k
{
  "cursor": {
    "page_size": 15,
    "has_more": false,
    "total": 1,
    "remaining_results": 0,
    "data": [
      {
        "txid": 0,
        "postings": [
          {
            "source": "world",
            "destination": "test:001",
            "amount": 100,
            "asset": "USD/2"
          }
        ],
        "reference": "foobar",
        "timestamp": "2021-10-21T23:39:28-04:00",
        "hash": "c516d65aa60b8272a6b1caac6355e3b899b72a070c0c01f64ebb581ba119b904",
        "metadata": {}
      }
    ]
  },
  "err": null,
  "ok": true
}

▶ curl -X POST http://localhost:3068/test-01/transactions/0/revert | jq .                                                                                    
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    11  100    11    0     0   3666      0 --:--:-- --:--:-- --:--:--  3666
{
  "ok": true
}

▶ curl -X GET http://localhost:3068/test-01/transactions | jq .                                                                                              
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   661  100   661    0     0   322k      0 --:--:-- --:--:-- --:--:--  322k
{
  "cursor": {
    "page_size": 15,
    "has_more": false,
    "total": 2,
    "remaining_results": 0,
    "data": [
      {
        "txid": 1,
        "postings": [
          {
            "source": "test:001",
            "destination": "world",
            "amount": 100,
            "asset": "USD/2"
          }
        ],
        "reference": "revert_foobar",
        "timestamp": "2021-10-21T23:39:54-04:00",
        "hash": "bc946b08061fc4c8076b009281f8a5104417498834bd94c61ddc6477334d949e",
        "metadata": {}
      },
      {
        "txid": 0,
        "postings": [
          {
            "source": "world",
            "destination": "test:001",
            "amount": 100,
            "asset": "USD/2"
          }
        ],
        "reference": "foobar",
        "timestamp": "2021-10-21T23:39:28-04:00",
        "hash": "c516d65aa60b8272a6b1caac6355e3b899b72a070c0c01f64ebb581ba119b904",
        "metadata": {
          "scheme/state": "reverted",
          "scheme/state/reverted-by": "1"
        }
      }
    ]
  },
  "err": null,
  "ok": true
}

henry-jackson and others added 3 commits October 21, 2021 23:40
Signed-off-by: Henry Jackson <henry.jackson.95@gmail.com>
Signed-off-by: Henry Jackson <henry.jackson.95@gmail.com>
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #61 (27c7379) into main (cbb50ef) will increase coverage by 1.77%.
The diff coverage is 51.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   34.01%   35.78%   +1.77%     
==========================================
  Files          26       28       +2     
  Lines        1379     1537     +158     
==========================================
+ Hits          469      550      +81     
- Misses        858      930      +72     
- Partials       52       57       +5     
Impacted Files Coverage Δ
api/http.go 0.00% <0.00%> (ø)
storage/postgres/transactions.go 0.00% <0.00%> (ø)
storage/storage.go 38.46% <ø> (ø)
ledger/ledger.go 72.86% <73.33%> (+0.06%) ⬆️
storage/sqlite/transactions.go 84.34% <89.65%> (+2.20%) ⬆️
core/metadata.go 100.00% <100.00%> (ø)
core/posting.go 100.00% <100.00%> (ø)
core/transaction.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff23024...27c7379. Read the comment docs.

henry-jackson and others added 4 commits October 22, 2021 22:51
Signed-off-by: Henry Jackson <henry.jackson.95@gmail.com>
Signed-off-by: Henry Jackson <henry.jackson.95@gmail.com>
ledger/ledger.go Outdated
return tx, err
}

meta, err := l.store.GetMeta("transaction", id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken the meta fetching should be already covered by in the store

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. I'll remove this

@@ -121,6 +121,38 @@ func NewHttpAPI(lc fx.Lifecycle, resolver *ledger.Resolver) *HttpAPI {
c.JSON(200, res)
})

r.POST("/:ledger/transactions/:id/revert", func(c *gin.Context) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about enclosing the logic here in a new function of ledger/ledger.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, probably too much business logic going on here, moved it

@henry-jackson
Copy link
Contributor Author

@altitude good to go now 🚀

@altitude altitude merged commit cb88643 into formancehq:main Oct 28, 2021
@altitude
Copy link
Member

Awesome, thanks @henry-jackson!

@henry-jackson henry-jackson deleted the revert-transaction branch October 29, 2021 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revert transaction
3 participants