-
Notifications
You must be signed in to change notification settings - Fork 186
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
Remove decommissioned networks #1880
Conversation
a5493cb
to
3cecdca
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1880 +/- ##
==========================================
- Coverage 78.30% 78.20% -0.10%
==========================================
Files 100 100
Lines 9126 9123 -3
==========================================
- Hits 7146 7135 -11
- Misses 1342 1348 +6
- Partials 638 640 +2 ☔ View full report in Codecov by Sentry. |
18d12b6
to
84d71ff
Compare
dccfc48
to
ac1a6eb
Compare
ac1a6eb
to
feedd25
Compare
feedd25
to
0d6bdf8
Compare
0887b2c
to
6de2b42
Compare
9eda3bc
to
3510edd
Compare
3510edd
to
7383569
Compare
@@ -443,125 +443,6 @@ func TestState(t *testing.T) { | |||
}) | |||
} | |||
|
|||
func TestEvents(t *testing.T) { |
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.
Is this test obsolete?
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.
I moved it into a possibly better place
Line 27 in c391201
func TestEvents(t *testing.T) { |
number: 1, | ||
chain: utils.Goerli, | ||
name: "goerli network (pre 0.7.0 without sequencer address)", | ||
}, |
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.
are you sure that these tests should be removed? because sequencerFallback is part of our logic in old hashes, maybe there is an alternative in mainnet ?
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.
I've found already 2 mainnet examples in this test if I am not mistaken
Lines 31 to 44 in 7383569
{ | |
// block 0: main | |
// https://alpha-mainnet.starknet.io/feeder_gateway/get_block?blockNumber=0 | |
number: 0, | |
chain: utils.Mainnet, | |
name: "mainnet (pre 0.7.0 without sequencer address)", | |
}, | |
{ | |
// block 833: main | |
// https://alpha-mainnet.starknet.io/feeder_gateway/get_block?blockNumber=833 | |
number: 833, | |
chain: utils.Mainnet, | |
name: "mainnet 833 (post 0.7.0 without sequencer address)", | |
}, |
{ | ||
number: 283364, | ||
chain: utils.Integration, | ||
name: "Block 283364 with Declare v2", |
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.
same, specific tx type with version
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.
These one I looked for but found only version v3 - let me run my scripts over the weekend 🤔
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.
added Sepolia block #7
@@ -204,12 +126,12 @@ func TestBlockHash(t *testing.T) { | |||
}) | |||
|
|||
t.Run("no error if block is unverifiable", func(t *testing.T) { |
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.
new version of this test doesn't test what it was intended to. Here is metadata of goerli:
UnverifiableRange: []uint64{119802, 148428},
119801 is beginning of unverifiable range
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.
If we don't need this logic anymore it's even better, but I'm not sure. cc @IronGauntlets
core/class_test.go
Outdated
gw := adaptfeeder.New(client) | ||
tests := []struct { | ||
classHash string | ||
checkNoCompiled bool | ||
}{ | ||
{ | ||
// https://external.integration.starknet.io/feeder_gateway/get_class_by_hash?classHash=0x1cd2edfb485241c4403254d550de0a097fa76743cd30696f714a491a454bad5 | ||
classHash: "0x1cd2edfb485241c4403254d550de0a097fa76743cd30696f714a491a454bad5", | ||
// https://alpha-mainnet.starknet.io/feeder_gateway/get_class_by_hash?classHash=<calss_hash> |
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.
missing class_hash
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.
The url will be the same besides the class_hash
parameter. I moved the comment above, and fixed the typo.
afabe59
to
1845f32
Compare
integGw := adaptfeeder.New(integClient) | ||
|
||
blockWithRevertedTxn, err := integGw.BlockByNumber(context.Background(), 304740) | ||
blockWithRevertedTxn, err := integGw.BlockByNumber(context.Background(), 19957) |
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.
I've found no difference in feeder response either from old & recents blocks.
So for these 2 new tests "reverted" & "v3 tx" I'm using the same block 19957
here
(v0.6 api) and in the following test
7b0024f
to
65a1c20
Compare
65a1c20
to
8bc8983
Compare
This pull request is stale because it has been open 35 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
Remove decommissioned networks:
Goerli
,Goerli2
andIntegration
.Fixes #1753 , #1758