Page MenuHomePhabricator

[ecash-agora] Partial Agora offers: Add SLP support to plugin, `Agora` and `AgoraOffer`
ClosedPublic

Authored by tobias_ruck on Sep 11 2024, 13:31.

Details

Summary

In D16743 script was added, and now we make it usable by both indexing these offers via the agora.py plugin, as well as adding support to accepting/cancelling them via the Agora class, and AgoraOffer, respectively.

Depends on D16743.

Test Plan

npm run integration-tests

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ecash-agora-partial-offer-slp
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30306
Build 60139: Build Diffecash-agora-integration-tests · ecash-agora-tests
Build 60138: arc lint + arc unit

Event Timeline

tobias_ruck retitled this revision from [ecash-agora] Partial Agora offers: Add support to plugin, `Agora` and `AgoraOffer` to [ecash-agora] Partial Agora offers: Add SLP support to plugin, `Agora` and `AgoraOffer`.Sep 11 2024, 13:51
modules/ecash-agora/tests/partial.slp.bigsats.test.ts
157

isn't this the 2nd output?

161

isn't this the third output?

244

same question as above -- what is outputs[0] ?

modules/ecash-agora/tests/partial.test.slp.ts
1

so we have partial.slp.test.ts and also partial.test.slp.ts ?

I can't really tell from the naming what I should expect to find in each file / why they are distinct.

modules/ecash-agora/tests/partial.slp.bigsats.test.ts
157

yeah well the OP_RETURN is the 0th output 🙃

You would've hoped there's a convention for this already, but I can change it for clarity

244

outputs[0] is the OP_RETURN
I can add a test that checks if its Script starts with OP_RETURN "SLP\0" if that helps :)

modules/ecash-agora/tests/partial.test.slp.ts
1

Good point—maybe call it partial-helpers-slp.ts, or something else

Fabien requested changes to this revision.Sep 17 2024, 10:04
Fabien added a subscriber: Fabien.
Fabien added inline comments.
modules/ecash-agora/src/agora.ts
206

no need for the else

266

dito

399

dito

596

Reviewer note: [0] is fine because the number of bytes is limited by design, and if > 255 it's going to be invalid

This revision now requires changes to proceed.Sep 17 2024, 10:04

re this comment

image.png (136×604 px, 12 KB)

modules/ecash-agora/src/agora.ts
206

I think the else has two good reasons:

  1. If a user specifies something other than ONESHOT and PARTIAL they should get an error (instead of falling into PARTIAL by default)
  2. If we add another variant, we can simply add another else if branch
399

same reason here

modules/ecash-agora/agora.py
399

image.png (500×958 px, 906 KB)

address review + based memes

This revision is now accepted and ready to land.Sep 24 2024, 19:55