Page MenuHomePhabricator
Differential D14067

[alias-server] Use ecash-script to parse alias registrations according to latest spec
ClosedPublic
Actions

Authored by bytesofman on Jun 15 2023, 15:56.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCdd10b7d79ade: [alias-server] Use ecash-script to parse alias registrations according to…
Summary

T3060

Responding to feedback of D13916. With ecash-script library developed and the spec finalized, implement here.

Key change is modifying the parseAliasTx function. This change requires new mocks, which requires changes to most unit test files.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
parse-revised-spec
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningapps/alias-server/src/alias.js:84SPELL1Possible Spelling Mistake
Unit
No Test Coverage
Build Status
Buildable 24059
Build 47728: Build Diffalias-server-tests
Build 47727: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jun 15 2023, 15:56
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 15 2023, 15:56
bytesofman requested review of this revision.Jun 15 2023, 15:56
Harbormaster completed remote builds in B24057: Diff 40811.Jun 15 2023, 15:57
bytesofman planned changes to this revision.EditedJun 15 2023, 15:57
  • Unit tests for txs with multiple OP_RETURN, fee good and fee bad
  • Unit tests for normal xec tx
  • Remove debug logging
bytesofman updated this revision to Diff 40814.Jun 15 2023, 16:53

More unit tests, remove debug logging

Harbormaster completed remote builds in B24059: Diff 40814.Jun 15 2023, 16:53
bytesofman updated this revision to Diff 40815.Jun 15 2023, 16:54

spelling linter

Harbormaster completed remote builds in B24060: Diff 40815.Jun 15 2023, 17:02
bytesofman mentioned this in D13916: [alias-server] Parse aliases according to latest spec.Jun 15 2023, 18:49
Fabien requested changes to this revision.Jun 16 2023, 13:20
Fabien added a subscriber: Fabien.
Fabien added inline comments.
apps/alias-server/src/alias.js
171 ↗(On Diff #40815)

But you did account for the fee despite the alias registration being invalid.
This is also worth a test to ensure this case is properly handled.

190 ↗(On Diff #40815)

The fees should be incremented at this time

This revision now requires changes to proceed.Jun 16 2023, 13:20
bytesofman updated this revision to Diff 40840.Jun 16 2023, 13:38

Increment fee only if alias is valid, add unit test to confirm

Harbormaster completed remote builds in B24076: Diff 40840.Jun 16 2023, 13:40
Fabien accepted this revision.Jun 16 2023, 19:12
This revision is now accepted and ready to land.Jun 16 2023, 19:12
Closed by commit rABCdd10b7d79ade: [alias-server] Use ecash-script to parse alias registrations according to… (authored by bytesofman). · Explain WhyJun 16 2023, 19:24
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCdd10b7d79ade: [alias-server] Use ecash-script to parse alias registrations according to….

Revision Contents
Changeset List

PathSize
apps/
alias-server/
constants/
6 lines
11 lines
1 line
src/
213 lines
test/
721 lines
8 lines
31 lines
10 lines
4 lines
mocks/
23 lines

Diff 40814

View Options

apps/alias-server/constants/alias.js

Loading...
View Options

apps/alias-server/package-lock.json

Loading...
View Options

apps/alias-server/package.json

Loading...
View Options

apps/alias-server/src/alias.js

Loading...
View Options

apps/alias-server/test/aliasTests.js

Loading...
View Options

apps/alias-server/test/chronikWsHandlerTests.js

Loading...
View Options

apps/alias-server/test/dbTests.js

Loading...
View Options

apps/alias-server/test/eventsTests.js

Loading...
View Options

apps/alias-server/test/mainTests.js

Loading...
View Options

apps/alias-server/test/mocks/aliasMocks.js

Loading...
View Options

apps/alias-server/test/mocks/unconfirmedAliasTxs.js

Loading...
View Options

apps/alias-server/test/utilsTests.js

Loading...