Page MenuHomePhabricator

more comprehensive proptype validation for transaction components
AbandonedPublic

Authored by bytesofman on Jan 11 2022, 04:05.

Details

Reviewers
darn-it-dan
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

Just some sanity checks for proptypes when it comes to transactions.
Since i'm new to the project, I had to map out all the data to really understand what was going on,
and I just decided to implement my findings into concrete code.

Test Plan

nothing much has changed,
since proptypes are only runtime validations:
so you can start up the wallet and make sure there is no proptype errors in the console.
make sure said wallet has a variety of transactions.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
tx-history-types
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17946
Build 35714: Build Diffcashtab-tests
Build 35713: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jan 11 2022, 04:05
bytesofman added a subscriber: bytesofman.

Please remove the code comments (answered above). Don't need them in the codebase.

web/cashtab/src/components/Wallet/Tx.js
501 ↗(On Diff #31741)

Looks like this can also be a buffer. I've created a task to move some of the parsing logic around so that it's always a string here.

508 ↗(On Diff #31741)

This is how they happen to be returned from the backend.

This revision now requires changes to proceed.Jan 11 2022, 22:31

Deleted comments that are not needed in codebase

bytesofman added inline comments.
web/cashtab/src/components/Wallet/Tx.js
500 ↗(On Diff #31786)

After D10821 lands, this will be a string. Please update.

This revision now requires changes to proceed.Jan 19 2022, 20:36

changed opReturnMessage to string

Please rebase to latest master to include the op_return prop type changes from D10821

git checkout master
git pull origin master
git checkout tx-history-types
git rebase master
arc diff

May need to resolve some conflicts as D10821 also changes the Tx.js file

This revision now requires changes to proceed.Jan 20 2022, 17:54
bytesofman requested changes to this revision.Feb 1 2022, 21:04

I would green this and add it -- waiting to hear from author on Telegram or here to confirm it can be landed appropriately. If not, will commandeer and add later.

This revision now requires changes to proceed.Feb 1 2022, 21:04
bytesofman abandoned this revision.
bytesofman edited reviewers, added: darn-it-dan; removed: bytesofman.

obsolete