Skip to content

refactor: use scheduler quotes for signing - #1125

Open
charmful0x wants to merge 4 commits into
feat/push-compute-authority-fast-schedulerfrom
feat/push-scheduler-quote
Open

refactor: use scheduler quotes for signing#1125
charmful0x wants to merge 4 commits into
feat/push-compute-authority-fast-schedulerfrom
feat/push-scheduler-quote

Conversation

@charmful0x

Copy link
Copy Markdown

this PR refactor moves the Arweave fee/anchor lookup out of dev_push, addressing #1101 (comment)

push asks the selected scheduler for a /quote. the arweave-scheduler@1.0 returns a commitment spec with the reward and anchor, and tx@1.0 build the native tx. authority selection still chooses the local signing wallet -- and the scheduler submit the signed tx

schedulers without /quote keep the existing codec negotiation (422)

live probe on Arweave:

Step Tx Block
P1 registration wbrLtPg0iUaUdhx4TgwRBNI679HhV3Gs-iK5o06ddvg 1997965
P2 registration DVM8Ixgrstc21jtzt_3hjnD_ZFj2QPIijXJWBiicyZA 1997965
input to P1 HShXV7WLNkH7ps-MiRZbY1lZoOCQ4CwNS3PKyxMfHxk 1997971
P1 -> P2 push T5cgP-3xpDZBJ6jmrRyAdAJo6qViG8VTOINqa3hd4pk 1997976

P1's /push produced the outgoing tx, and P2 computed it at slot 1 with the expected message and from-process

{error,
#{
<<"status">> := 422,
<<"require-codec">> := <<"tx@1.0">>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any reason not to just return the message with the commitment-spec already at the outer wrapper? I think you can also just say field-reward and maybe field-anchor, which the tx@1.0 device will already pick up and honor in the correct place.

<<"require-codec">> := <<"tx@1.0">>,
<<"commitment-spec">> := #{
<<"commitment-device">> := <<"tx@1.0">>,
<<"tx-header">> := #{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hopefully the above would render the tx-header here unnecessary -- we just take the result and hb_message:commit(MsgToAssign, QuoteRes, Opts)?

{error,
#{
<<"status">> := 422,
<<"require-codec">> := <<"tx@1.0">>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment ordering is getting crazy here, but if we do the other two comments in this zone, then I think require-codec can essentially be removed from the downgrade path. That might require some tweaks elsewhere but should be contained if so. Benefit is that the response is a straightforward commitment spec we can just hb_message:commit with

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

agreed 100%, great points, working on it!

Comment thread src/preloaded/process/dev_push.erl Outdated
NormMsg = normalize_message(AugmentedMsg, Opts),
SignedNormMsg = sign_result(NormMsg, TargetProcess, Codec, Opts),
NormMsg = normalize_message(MsgToPush, Opts),
SignedNormMsg = apply_security(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fmt

Comment on lines +677 to +681
hb_ao:resolve(
{as, <<"process@1.0">>, TargetProcess},
#{ <<"path">> => <<"as">>, <<"as">> => <<"scheduler">> },
QuoteOpts
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is making me lean more towards using just HEAD /schedule for that call? Then your whole flow here (including the device info part) is just HEAD TargetProc/schedule. If it specifies a commitment-device then you know you can use it as a commitment spec, if it doesn't do then we do the default flow.

Comment thread src/preloaded/process/dev_push.erl Outdated
Comment on lines +682 to +683
Info = hb_device:info(Scheduler, QuoteOpts),
true ?= lists:member(<<"quote">>, maps:get(exports, Info, [])),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We generally only use hb_device in the kernel if possible. I think here (the above route notwithstanding) the idea would just be to try the /quote and match on {ok, Quote} ?= ...

Comment thread src/preloaded/process/dev_push.erl Outdated
{ToSign, CommitSpec} =
case Spec of
_ when is_map(Spec) ->
{normalize_message(Msg, Opts),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fmt

@charmful0x

Copy link
Copy Markdown
Author

thanks for the feedback @samcamwilliams -- in latest commit i addressed the HEAD TargetProc/schedule feature:

  • HEAD TargetProc/schedule replaces /quote and device inspection
  • HEAD and retryable 422 responses return flat commitment specs, passed directly into signing
  • removed scheduling’s expr require-codec (introduced in feat/push-compute-authority-fast-scheduler - base branch) and nested commitment-spec / tx-header wrappers.
  • kept the existing wallet selection, one-winston/data-free txs, message tags, and legacy httpsig -> ans104 fallback

the flow is (pseudocode):

head = HEAD TargetProc/schedule
if head failed:
    return error

spec = head if head has commitment-device else default_spec

// existing security policy chooses the local signing wallet(s)
// and calls hb_message:commit(Message, WalletOpts, Spec)
signed = apply_security(message, TargetProc, spec, opts)
result = POST TargetProc/schedule with signed

if result.status == 422 and spec == default_spec:
    if result has commitment-device:
        retry_spec = result
    else if spec == httpsig@1.0:
        retry_spec = ans104@1.0
    else:
        return result

    if retry_spec != spec:
        signed = apply_security(message, TargetProc, retry_spec, opts)
        return POST TargetProc/schedule with signed

return result

so HEAD without a commitment device keeps the default flow, and a retry uses the whole flat 422 response as the spec. the retry is bounded -- the scheduler still just supplies the native fields and submits the signed tx

live probed again on Arweave L1:

Step Tx Block
P1 registration BaGqiKlP_PWbrYZaeRDY0nk4K01ZkJaKr1DB9zH_0OA 1998635
P2 registration XYwQQf3AYSzQbUnZJdLdkuPdlb1HZG1lQpZHGMJ9Os4 1998635
input to P1 rhX279NB_1H3oR_N0IPuHWLcafh3ONcpThk0w1YiqvA 1998638
P1 → P2 push 56NGd2zOf5QTMLmsohz4ybtsmftmlvTgC4Yl6M3bVAk 1998640

Comment thread src/preloaded/codec/dev_tx.erl Outdated
{ok, TX} = to(hb_private:reset(Msg), Req, Opts),
TABM = hb_private:reset(Msg),
TX0 =
case hb_util:int(hb_maps:get(<<"field-data_size">>, Req, -1, Opts)) of

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this needed out of interest?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

replaced the separate header-building path with normal shared tx encoding and explicit validation

Comment thread src/preloaded/codec/dev_tx.erl Outdated
dev_tx_to:fields_to_tx(TX, ?FIELD_PREFIX, Fields, Opts).

%% @doc Encode message tags when the signing request specifies zero data size.
header(TABM, Opts) ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similar question to above -- necessary now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed it

Comment on lines +101 to +108
<<"commitment-device">> => <<"tx@1.0">>,
<<"field-format">> => 2,
<<"field-target">> => Target,
<<"field-quantity">> => 1,
<<"field-reward">> => Reward,
<<"field-anchor">> => hb_util:encode(Anchor),
<<"field-data_size">> => 0,
<<"field-data_root">> => <<>>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IIRC don't most/all of the keys already use the fields by default? Or is that not how it works? If not, then the thing to check would be that they are promoted to first-class keys in the base message upon decode normally.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes they are -- i reused that mapping and added decode assertions

<<"field-quantity">> => 1,
<<"field-reward">> => Reward,
<<"field-anchor">> => hb_util:encode(Anchor),
<<"field-data_size">> => 0,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I get where we are trying to go here but any ideas about a nicer root? We are just trying to communicate that we shouldn't include a TX data body? Is the plan to pick that up in dev_tx on the other node and throw if the user tries to sign where the spec says there must be data-size: 0?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, now it throw before signing when the encoded tx contain payload data or nonzero size/root metadata

Comment thread src/preloaded/process/dev_scheduler.erl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants