Skip to content
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

all: Merge Horizon's Protocol 19 implementation into master #4340

Merged
merged 22 commits into from
Apr 19, 2022

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Apr 14, 2022

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    semver, or if it's mainly a patch change. The PR is targeted at the next
    release branch if it's not a patch change.

What

This is a final merge of all of Horizon's Protocol 19 changes into the mainline.

Why

See #4261 for a comprehensive list of features and related PRs for this merge.

Known limitations

n/a

Shaptic and others added 18 commits March 18, 2022 13:31
…#4279)

* Add Protocol 19 XDR: raw and generated
* Update codebase to use new XDR for predicates/timebounds
* Add stringification method for Preconditions
* Add encode/decode helper for CAP-40 signed payloads
* Add Horizon DB migration: signers can be longer now
* Disable generating the V3 extension on accounts
horizon: Merge master into horizon-protocol-19 branch
Adds support for new CAP-21 preconditions with a simple, but breaking API
change:

TransactionParams.Timebounds -> TransactionParams.Preconditions.TimeBounds

You can now pass a lot more preconditions to a Transaction (including
timebounds), and these are managed by the new Preconditions object. All of the
XDR abstractions are hidden away behind this object.
* Add cap-21 transaction preconditions

* Replace xdr.TransactionEnvelope.TimeBounds -> .Preconditions function

* Rename Signers -> ExtraSigners for more consistent naming

* Store the new precondition fields in the DB

* Render the new precondition fields in the resourceadapter

* ./gogenerate.sh

* use a pointer so omitempty works

* more consistent naming

* Keep Timebounds helper

* PR feedback

* tidy transaction precondition helpers

* typo fixes

* remove dead param

* use bigint so we have enough space for a uint32

* use text instead of varchar, to avoid future migrations

* Updating scenario sqls

* need to load the preconditions from the db

* Temporary passing data for v2 preconditions transaction insertion test

* Update xdr/transaction_envelope_test.go

Co-authored-by: George <[email protected]>

* Update services/horizon/internal/db2/history/transaction_ledger_bounds.go

Co-authored-by: George <[email protected]>

* PR feedback

Co-authored-by: George <[email protected]>
…s`. (#4307)

* Timebounds -> TimeBounds, Ledgerbounds -> LedgerBounds
* Duration -> int64, SignerKey -> string
* Update XDR to latest from sisuresh/stellar-core#cap21-40-2
* Update codebase to make cb predicates use Int64s again
horizon: Merge master into horizon-protocol-19 branch
Added additional round trip verification by re-querying preconditions on tx after submission, per pr feedback
services/horizon/integration: Added more integration tests for coverage of preconditions on transactions
…years (#4337)

* Merge Protocol 19 migrations into a single file
* Actually generate V3 account extensions
* Change accounts.sequence_time to be bigint over timestamp:

This is necessary because PostgreSQL puts "reasonable" limitations
on how dates can be represented. Because the state verifier test
will generate random int64s for the V3 account extension field
introduced in CAP-21, this will blow up on inserting into the DB
with errors like:

    pq: date/time field value out of range: "120911623444-02-01 21:35:20Z"

because PostgreSQL only allows years up to 5874897
(https://stackoverflow.com/a/36446977).

Technically, since this field is not user-controlled and comes
directly from Stellar Core, we would not see this in practice.

However, it's a more durable fix to stop the extra layer of
"interpretation" of this value and just treat it like it is:
a 64-bit integer that happens to represent a UNIX timestamp.
@Shaptic Shaptic requested a review from a team April 14, 2022 16:34
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

🎉 Looks good to me. One minor comment, but it doesn't need to block anything. I mostly looked at the SDK changes, and defer to @stellar/horizon-committers for the Horizon changes.

protocols/horizon/main.go Outdated Show resolved Hide resolved
…nt` to string (#4339)

* `minSeqAge` can be unsigned in the XDR
* Update codebase to conform to new data type: null.Int -> null.String
* Fix time-based tests to be tz-agnostic + work w/ uint64
@@ -0,0 +1,28 @@
-- +migrate Up
Copy link
Contributor

Choose a reason for hiding this comment

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

do any of these schema changes require extra notes on CHANGELOG for state rebuild or re-ingestion requirements? I noticed the services/horizon/CHANGELOG.md isn't updated for P19 yet, that was gonna be drated up on the release branch instead?

Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

did a skim review, looks good, left one comment on the db migrations, just as far as any notes that may need to be raised in CHANGELOG based on state/re-ingestion requirements if needed. would be good for others to skim this too. great work!

@Shaptic Shaptic merged commit 9f968df into master Apr 19, 2022
@erika-sdf erika-sdf deleted the horizon-protocol-19 branch May 19, 2022 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants