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

Remove frozen ABI modules from solana-sdk #13008

Merged
merged 1 commit into from
Oct 20, 2020
Merged

Conversation

mvines
Copy link
Member

@mvines mvines commented Oct 20, 2020

With the creation of solana-program-sdk (#12989), the frozen ABI support can no longer live in solana-sdk as that would cause a circular dependency. Create a stand-alone /frozen-abi (and /frozen-abi-macro) crate for this facility

@mvines mvines added the v1.4 label Oct 20, 2020
@mvines mvines force-pushed the abi branch 7 times, most recently from 2d5d276 to e1d2bfb Compare October 20, 2020 07:07
@@ -213,9 +213,7 @@ atomic_example_impls! { AtomicI64 }
atomic_example_impls! { AtomicIsize }
atomic_example_impls! { AtomicBool }

#[cfg(feature = "everything")]
Copy link
Member Author

Choose a reason for hiding this comment

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

Feature removal!

@@ -550,7 +550,7 @@ mod tests {
test_field2: i8,
}

#[frozen_abi(digest = "Hv597t4PieHYvgiXnwRSpKBRTWqteUS4nHZHY6ZxX69v")]
#[frozen_abi(digest = "GMeECsxg37a5qznstWXeeX3d6HXs6j12oB4SKaZZuNJk")]
Copy link
Member Author

Choose a reason for hiding this comment

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

@ryoqun - These two hashes changed on me for some reason. Seems harmless though. Maybe it's due to the crate rename somenow?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, it detects such renames because it need to consider XX::Hash to be different from YY::Hash.

$ diff -U3 /tmp/abi8/*Hv597t4PieHYvgiXnwRSpKBRTWqteUS4nHZHY6ZxX69v* /tmp/abi8/*GMeECsxg37a5qznstWXeeX3d6HXs6j12oB4SKaZZuNJk*
--- /tmp/abi8/abi_digester__tests__TestNest_frozen_abi__test_abi_digest_Hv597t4PieHYvgiXnwRSpKBRTWqteUS4nHZHY6ZxX69v    2020-10-20 21:17:01.936993559 +0900
+++ /tmp/abi8/abi_digester__tests__TestNest_frozen_abi__test_abi_digest_GMeECsxg37a5qznstWXeeX3d6HXs6j12oB4SKaZZuNJk    2020-10-20 21:21:09.115486766 +0900
@@ -1,31 +1,31 @@
 struct TestNest (fields = 1)
-    field nested_field: [solana_sdk::abi_digester::tests::TestStruct; 5]
+    field nested_field: [solana_frozen_abi::abi_digester::tests::TestStruct; 5]
         tuple (elements = 5)
-            element solana_sdk::abi_digester::tests::TestStruct
+            element solana_frozen_abi::abi_digester::tests::TestStruct
                 struct TestStruct (fields = 2)
                     field test_field: i8
                         primitive i8
                     field test_field2: i8
                         primitive i8
-            element solana_sdk::abi_digester::tests::TestStruct
+            element solana_frozen_abi::abi_digester::tests::TestStruct
                 struct TestStruct (fields = 2)
                     field test_field: i8
                         primitive i8
                     field test_field2: i8
                         primitive i8
-            element solana_sdk::abi_digester::tests::TestStruct
+            element solana_frozen_abi::abi_digester::tests::TestStruct
                 struct TestStruct (fields = 2)
                     field test_field: i8
                         primitive i8
                     field test_field2: i8
                         primitive i8
-            element solana_sdk::abi_digester::tests::TestStruct
+            element solana_frozen_abi::abi_digester::tests::TestStruct
                 struct TestStruct (fields = 2)
                     field test_field: i8
                         primitive i8
                     field test_field2: i8
                         primitive i8
-            element solana_sdk::abi_digester::tests::TestStruct
+            element solana_frozen_abi::abi_digester::tests::TestStruct
                 struct TestStruct (fields = 2)
                     field test_field: i8
                         primitive i8
$ sha256sum /tmp/abi8/abi_digester__tests__TestNest_frozen_abi__test_abi_digest_Hv597t4PieHYvgiXnwRSpKBRTWqteUS4nHZHY6ZxX69v | xxd -r -p - | base58 && echo
Hv597t4PieHYvgiXnwRSpKBRTWqteUS4nHZHY6ZxX69v

$ sha256sum /tmp/abi8/abi_digester__tests__TestNest_frozen_abi__test_abi_digest_GMeECsxg37a5qznstWXeeX3d6HXs6j12oB4SKaZZuNJk | xxd -r -p - | base58 && echo
GMeECsxg37a5qznstWXeeX3d6HXs6j12oB4SKaZZuNJk

@@ -262,7 +262,7 @@ mod test_bank_serialize {

// These some what long test harness is required to freeze the ABI of
// Bank's serialization due to versioned nature
#[frozen_abi(digest = "5rd8RyVSLH3hm12xJDVCJWgc1gyqb4Ukt2hJLJNfsB5v")]
#[frozen_abi(digest = "ULV2jDndxR3JB677ayyjaamtAcZ24q75tCkHS2bKVoy")]
Copy link
Member Author

Choose a reason for hiding this comment

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

This hash changed too. Again seems harmless but 🤔

Copy link
Member

Choose a reason for hiding this comment

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

yeah, because of fully qualified symbol name change of frozen abi:

$ diff -U3 /tmp/abi8/*5rd8RyVSLH3hm12xJDVCJWgc1gyqb4Ukt2hJLJNfsB5v* /tmp/abi8/*ULV2jDndxR3JB677ayyjaamtAcZ24q75tCkHS2bKVoy*
--- /tmp/abi8/serde_snapshot__tests__test_bank_serialize__BankAbiTestWrapperFuture_frozen_abi__test_abi_digest_5rd8RyVSLH3hm12xJDVCJWgc1gyqb4Ukt2hJLJNfsB5v     2020-10-20 21:17:01.836997400 +0900
+++ /tmp/abi8/serde_snapshot__tests__test_bank_serialize__BankAbiTestWrapperFuture_frozen_abi__test_abi_digest_ULV2jDndxR3JB677ayyjaamtAcZ24q75tCkHS2bKVoy      2020-10-20 21:21:11.131409159 +0900
@@ -1617,11 +1617,11 @@
                         struct MessageProcessor (fields = 0)
             element solana_runtime::serde_snapshot::SerializableAccountsDB<solana_runtime::serde_snapshot::future::Context>
                 tuple (elements = 4)
-                    element solana_runtime::serde_snapshot::utils::serialize_iter_as_map::SerializableMappedIterator<core::iter::adapters::Map<core::slice::Iter<alloc::vec::Vec<alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>>>, <solana_runtime::serde_snapshot::future::Context as solana_runtime::serde_snapshot::TypeContext>::serialize_accounts_db_fields<solana_sdk::abi_digester::AbiDigester>::{{closure}}>>
+                    element solana_runtime::serde_snapshot::utils::serialize_iter_as_map::SerializableMappedIterator<core::iter::adapters::Map<core::slice::Iter<alloc::vec::Vec<alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>>>, <solana_runtime::serde_snapshot::future::Context as solana_runtime::serde_snapshot::TypeContext>::serialize_accounts_db_fields<solana_frozen_abi::abi_digester::AbiDigester>::{{closure}}>>
                         map (entries = 1)
                             key u64
                                 primitive u64
-                            value solana_runtime::serde_snapshot::utils::serialize_iter_as_seq::SerializableSequencedIterator<core::iter::adapters::Map<core::slice::Iter<alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>>, <solana_runtime::serde_snapshot::future::Context as solana_runtime::serde_snapshot::TypeContext>::serialize_accounts_db_fields<solana_sdk::abi_digester::AbiDigester>::{{closure}}::{{closure}}>>
+                            value solana_runtime::serde_snapshot::utils::serialize_iter_as_seq::SerializableSequencedIterator<core::iter::adapters::Map<core::slice::Iter<alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>>, <solana_runtime::serde_snapshot::future::Context as solana_runtime::serde_snapshot::TypeContext>::serialize_accounts_db_fields<solana_frozen_abi::abi_digester::AbiDigester>::{{closure}}::{{closure}}>>
                                 seq (elements = 1)
                                     element solana_runtime::serde_snapshot::future::SerializableAccountStorageEntry
                                         struct SerializableAccountStorageEntry (fields = 2)

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect, thanks for confirming!

@@ -573,7 +573,7 @@ mod tests {
VARIANT2(u8, u16),
}

#[frozen_abi(digest = "CKxzv7VjyUrNR9fGJpTpKyMBWJM4gepKshCS8oV14T1Q")]
#[frozen_abi(digest = "DywMfwKq8HZCbUfTwnemHWMN8LvMZCvipQuLddQ2ywwG")]
Copy link
Member

Choose a reason for hiding this comment

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

here it is:

$ diff -U3 /tmp/abi8/*CKxzv7VjyUrNR9fGJpTpKyMBWJM4gepKshCS8oV14T1Q* /tmp/abi8/*DywMfwKq8HZCbUfTwnemHWMN8LvMZCvipQuLddQ2ywwG*
--- /tmp/abi8/abi_digester__tests__TestVecEnum_frozen_abi__test_abi_digest_CKxzv7VjyUrNR9fGJpTpKyMBWJM4gepKshCS8oV14T1Q 2020-10-20 21:17:02.036989718 +0900
+++ /tmp/abi8/abi_digester__tests__TestVecEnum_frozen_abi__test_abi_digest_DywMfwKq8HZCbUfTwnemHWMN8LvMZCvipQuLddQ2ywwG 2020-10-20 21:21:09.171484610 +0900
@@ -1,7 +1,7 @@
 struct TestVecEnum (fields = 1)
-    field enums: alloc::vec::Vec<solana_sdk::abi_digester::tests::TestTupleVariant>
+    field enums: alloc::vec::Vec<solana_frozen_abi::abi_digester::tests::TestTupleVariant>
         seq (elements = 1)
-            element solana_sdk::abi_digester::tests::TestTupleVariant
+            element solana_frozen_abi::abi_digester::tests::TestTupleVariant
                 enum TestTupleVariant (variants = 2)
                     variant(0) VARIANT1 (fields = 2)
                         field u8

Copy link
Member

Choose a reason for hiding this comment

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

$ sha256sum /tmp/abi8/abi_digester__tests__TestVecEnum_frozen_abi__test_abi_digest_CKxzv7VjyUrNR9fGJpTpKyMBWJM4gepKshCS8oV14T1Q | xxd -r -p - | base58 && echo
CKxzv7VjyUrNR9fGJpTpKyMBWJM4gepKshCS8oV14T1Q

$ sha256sum /tmp/abi8/abi_digester__tests__TestVecEnum_frozen_abi__test_abi_digest_DywMfwKq8HZCbUfTwnemHWMN8LvMZCvipQuLddQ2ywwG | xxd -r -p - | base58 && echo
DywMfwKq8HZCbUfTwnemHWMN8LvMZCvipQuLddQ2ywwG

@ryoqun
Copy link
Member

ryoqun commented Oct 20, 2020

thanks for caring the frozen abi thing. :)

@mvines mvines force-pushed the abi branch 4 times, most recently from 0a8d566 to eb87ab3 Compare October 20, 2020 18:05
@codecov
Copy link

codecov bot commented Oct 20, 2020

Codecov Report

Merging #13008 into master will increase coverage by 0.0%.
The diff coverage is 70.8%.

@@           Coverage Diff           @@
##           master   #13008   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         362      364    +2     
  Lines       85383    85390    +7     
=======================================
+ Hits        70052    70070   +18     
+ Misses      15331    15320   -11     

@mvines mvines merged commit 6858950 into solana-labs:master Oct 20, 2020

here=$(dirname "$0")
set -x
exec ${here}/cargo nightly test --lib -- test_abi_
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this slipped through the shellcheck on this pr's ci build... #13039

Copy link
Member

Choose a reason for hiding this comment

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

well, shellcheck wasn't run at all. maybe there is a bug when dynamic buildpipeline.... I'll look at this later.

Copy link
Member

Choose a reason for hiding this comment

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

mergify bot added a commit that referenced this pull request Oct 21, 2020
* Remove frozen ABI modules from solana-sdk

(cherry picked from commit 6858950)

# Conflicts:
#	Cargo.lock
#	core/Cargo.toml
#	frozen-abi/macro/Cargo.toml
#	programs/bpf/Cargo.lock
#	programs/stake/Cargo.toml
#	programs/vote/Cargo.toml
#	runtime/Cargo.toml
#	sdk/Cargo.toml
#	version/Cargo.toml

* rebase

* fix broken ci (#13039)

Co-authored-by: Michael Vines <[email protected]>
Co-authored-by: Ryo Onodera <[email protected]>
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