-
Notifications
You must be signed in to change notification settings - Fork 9
multi: harden directed send and add receiver notification #225
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
Changes from all commits
cc9b1b6
126c699
8861e56
6c2fefa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,3 +38,4 @@ DS_Store | |
| go.work.sum | ||
|
|
||
| /tools/custom-gcl | ||
| .reviews/ | ||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package darepod | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/lightninglabs/darepo-client/db" | ||
| "github.com/lightninglabs/darepo-client/vtxo" | ||
| ) | ||
|
|
||
| // ownedScriptLookupAdapter wraps db.OORArtifactPersistenceStore to | ||
| // satisfy the vtxo.OwnedScriptLookup interface. It converts the | ||
| // db-specific record type to the vtxo-level OwnedReceiveScript. | ||
| type ownedScriptLookupAdapter struct { | ||
| store *db.OORArtifactPersistenceStore | ||
| } | ||
|
|
||
| // LookupOwnedReceiveScript delegates to the underlying store and | ||
| // converts the result to a vtxo.OwnedReceiveScript. | ||
| func (a *ownedScriptLookupAdapter) LookupOwnedReceiveScript( | ||
| ctx context.Context, | ||
| pkScript []byte) (*vtxo.OwnedReceiveScript, error) { | ||
|
|
||
| rec, err := a.store.LookupOwnedReceiveScript(ctx, pkScript) | ||
| if err != nil { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we could specifically only skip not-found errors (or even just log) otherwise surface the error.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. codex: Returning |
||
| return nil, err | ||
| } | ||
|
|
||
| return &vtxo.OwnedReceiveScript{ | ||
| ClientKey: rec.ClientKey, | ||
| OperatorPubKey: rec.OperatorPubKey, | ||
| ExitDelay: rec.ExitDelay, | ||
| }, nil | ||
| } | ||
|
|
||
| var _ vtxo.OwnedScriptLookup = (*ownedScriptLookupAdapter)(nil) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| package darepod | ||
|
|
||
| import ( | ||
| "context" | ||
| "database/sql" | ||
| "errors" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/btcsuite/btcd/btcec/v2" | ||
| "github.com/lightninglabs/darepo-client/db" | ||
| "github.com/lightninglabs/darepo-client/round" | ||
| fn "github.com/lightningnetwork/lnd/fn/v2" | ||
| "github.com/lightningnetwork/lnd/keychain" | ||
| ) | ||
|
|
||
| // ownedScriptCheckerAdapter implements round.OwnedScriptChecker by | ||
| // looking up pkScripts in the owned_receive_scripts persistence store. | ||
| type ownedScriptCheckerAdapter struct { | ||
| store *db.OORArtifactPersistenceStore | ||
| } | ||
|
|
||
| var _ round.OwnedScriptChecker = (*ownedScriptCheckerAdapter)(nil) | ||
|
|
||
| // IsOwnedScript returns whether the pkScript is registered as an owned | ||
| // receive script in the OOR artifact store. Returns an error for real | ||
| // store failures; a not-found result returns false with no error. | ||
| func (a *ownedScriptCheckerAdapter) IsOwnedScript(ctx context.Context, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: missing godoc. |
||
| pkScript []byte) fn.Result[bool] { | ||
|
|
||
| if a.store == nil { | ||
| return fn.Ok(false) | ||
| } | ||
|
|
||
| // Use a context that survives cancellation so the DB lookup | ||
| // completes even if the caller's context is being torn down | ||
| // (e.g., during round confirmation in a shutting-down FSM). | ||
| lookupCtx := context.WithoutCancel(ctx) | ||
|
|
||
| _, err := a.store.LookupOwnedReceiveScript(lookupCtx, pkScript) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is correct as is, we should instead surface the error and a bool flag if it returns the receive script? |
||
| if err != nil { | ||
| // Not-found means the script isn't ours. | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return fn.Ok(false) | ||
| } | ||
|
|
||
| return fn.Err[bool](fmt.Errorf( | ||
| "lookup owned receive script: %w", err, | ||
| )) | ||
| } | ||
|
|
||
| return fn.Ok(true) | ||
| } | ||
|
|
||
| // ownedScriptRegistrarAdapter implements round.OwnedScriptRegistrar by | ||
| // persisting pkScripts in the owned_receive_scripts table. | ||
| type ownedScriptRegistrarAdapter struct { | ||
| store *db.OORArtifactPersistenceStore | ||
| operatorKey *btcec.PublicKey | ||
| exitDelay uint32 | ||
| } | ||
|
|
||
| var _ round.OwnedScriptRegistrar = (*ownedScriptRegistrarAdapter)(nil) | ||
|
|
||
| // RegisterOwnedScript persists the pkScript as a locally owned receive | ||
| // script in the OOR artifact store. | ||
| func (a *ownedScriptRegistrarAdapter) RegisterOwnedScript( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: missing godoc |
||
| ctx context.Context, pkScript []byte, | ||
| ownerKey keychain.KeyDescriptor) error { | ||
|
|
||
| if a.store == nil { | ||
| return fmt.Errorf("store is nil") | ||
| } | ||
|
|
||
| return a.store.UpsertOwnedReceiveScript( | ||
| ctx, db.OwnedReceiveScriptRecord{ | ||
| PkScript: pkScript, | ||
| ClientKey: ownerKey, | ||
| OperatorPubKey: a.operatorKey, | ||
| ExitDelay: int64(a.exitDelay), | ||
| Source: db.OwnedReceiveScriptSourceWallet, | ||
| CreatedAt: time.Now(), | ||
| LastUsedAt: fn.None[time.Time](), | ||
| }, | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -635,11 +635,21 @@ func (r *RPCServer) SendVTXO(ctx context.Context, | |
| return nil, err | ||
| } | ||
|
|
||
| // TODO(#241): Tune this cap based on round tree constraints | ||
| // and consider making it configurable. | ||
| const maxRecipients = 256 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should make an issue to tune this param later. |
||
|
|
||
| if len(req.Recipients) == 0 { | ||
| return nil, status.Errorf(codes.InvalidArgument, | ||
| "at least one recipient is required") | ||
| } | ||
|
|
||
| if len(req.Recipients) > maxRecipients { | ||
| return nil, status.Errorf(codes.InvalidArgument, | ||
| "too many recipients: %d (max %d)", | ||
| len(req.Recipients), maxRecipients) | ||
| } | ||
|
|
||
| // Resolve each recipient's pkScript and client pubkey from | ||
| // the proto Output destination. | ||
| recipients := make( | ||
|
|
@@ -656,14 +666,24 @@ func (r *RPCServer) SendVTXO(ctx context.Context, | |
| ) | ||
| } | ||
|
|
||
| if out.AmountSat <= 0 { | ||
| if out.AmountSat <= 0 || | ||
| out.AmountSat > int64(btcutil.MaxSatoshi) { | ||
|
|
||
| return nil, status.Errorf( | ||
| codes.InvalidArgument, | ||
| "recipient %d: amount must be "+ | ||
| "positive", i, | ||
| "between 1 and %d", | ||
| i, int64(btcutil.MaxSatoshi), | ||
| ) | ||
| } | ||
|
|
||
| // Overflow-safe addition. | ||
| if totalAmount > int64(btcutil.MaxSatoshi)-out.AmountSat { | ||
| return nil, status.Errorf( | ||
| codes.InvalidArgument, | ||
| "total amount overflows max supply") | ||
| } | ||
|
|
||
| pkScript, clientKey, err := r.resolveRecipientOutput( | ||
| out, | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should use an enum/oneof here to add more structure to the proto? So we can more easily distinguiish if this is a new VTXO from an in round send, or an oor.